detekt ReturnCount. Extracting the pruning and the is-this-an-episode
predicate reads better than suppressing it, and the cooldown rule now
has a name and a docstring of its own.
Two changes for the Sonos stutter the operator describes as rapid
play-pause-play at the start of a track.
The measurable one: nothing in the diagnostics could see it.
player_state records source/loading/error but not whether we are
playing, track_change needs the queue index to move, and the heartbeat
samples once every 45s. A few seconds of oscillation that changes no
index fell through all three, which is why the symptom has been
described repeatedly and measured never. The poll loop now publishes
raw GetTransportInfo readings on change, and TransportFlapDetector
turns a burst of them into one summary event carrying the sequence
alongside local-vs-Sonos track and position -- enough to tell a cursor
disagreement from the renderer rebuffering. It samples at the 1Hz poll
cadence, so a faster oscillation lands aliased; that still answers
whether the renderer is leaving PLAYING, which is the open question.
The suspect one: during a cast the wrapped ExoPlayer was paused, not
stopped. pause() is only playWhenReady=false -- LoadControl keeps
loading, so the phone went on downloading the track the renderer was
streaming, over the same WiFi, re-arming at every track change via
syncLocalCursorToRemote's seekTo. At FLAC bitrates that is a second
full-rate download competing with the speaker, beginning exactly when
a new track does. stop() ends it; Media3 keeps media items, index and
position, and getPlaybackState() already reports STATE_READY while
remote, so cursor sync and handoff are unaffected. The route teardown
re-prepares for local playback.
Whether that download is the cause is unproven -- hence the
instrument landing alongside it rather than after it.
detekt flagged OutputPickerController as LargeClass once the verify
path landed. Extracting rather than suppressing: how the renderer's
queue is shaped is a different concern from which route is selected,
and it had grown big enough to hide a bug — every write in here is a
SOAP call that can fail on its own, and nothing ever read the result
back.
SonosQueueLoader now owns load / extend / verify / append and the
incremental diff. The picker keeps route selection and asks it for
queue work. No behaviour change.
The renderer's queue was written and never read back. loadQueueOnSonos
background-appends the tail one AddURIToQueue at a time and gives up
after 3 consecutive failures; Sonos rate-limits burst adds, so that
happens. The renderer was then left holding fewer tracks than we
believed, played what it actually had, and stopped — which looked
exactly like playback dying for no reason.
GetMediaInfo's NrTracks is the cheap authoritative answer and was not
being asked for anywhere in the app. Now:
- verifyQueueLength after every load (including when there is no tail
to append — the initial batch can be dropped the same way), appending
what the renderer is missing, bounded at 2 passes.
- RemoteStallWatchdog gains QueueState, so a stop is classified rather
than assumed: a stream that died resumes, a truncated queue gets
repaired at the next track, and a queue that simply ended does
nothing at all.
That last case was a bug shipped in #2700: the normal end of a queue is
a confirmed STOPPED with play intent, so every cast session would have
ended with three resume attempts and a `stalled` error for playback
that finished perfectly. No test described the end of a queue, so CI
had nothing to catch it with.
Queue reads are gated on the transport being stopped and cached for 5s,
so this never becomes a third SOAP call per second.
Filed as a product decision, but the code had already made it: the genre
queries filter tracks.missing_since inside their EXISTS, so an album
whose every file had gone was ALREADY absent from genre while still
listed under its year — where opening it found nothing playable. The two
browse axes disagreed, and whichever answer won, one of them had to
change.
Hiding is the answer. Browsing is how you go looking for something to
play, and the rule for that case is to take it out of view; the admin
missing-files surface is where absence gets reported, with far more
detail than a silent gap in a grid. It also means changing the axis that
was inconsistent rather than the one that was already right.
All three year queries move together — index, list and count. That is
the invariant #367 needed care for at the genre level: if the index
groups differently from the filter, a year leads to an empty page, and
if the count disagrees with the list then "Load more" promises rows that
never arrive.
The predicate is "has at least one playable track", which also excludes
an album carrying no tracks at all. Same answer for the same reason —
nothing to play, nothing to browse to — and it is what genre has always
done, since an album with no tracks contributes no genres either.
That last part changed two existing tests, which had been seeding
trackless albums as a convenience. Their intent (undated albums never
appear in a range) is untouched; they now seed a track each, which is
what a real album looks like anyway. Two new tests pin the actual
behaviour: a fully-missing album leaves the axis while a half-missing
one stays, and the count agrees with the filtered list.
The queue-filter test built TrackRefs without them; they have no
defaults, so compileDebugUnitTestKotlin failed. Caught by CI on the
Android lane while I was reading the Go one.
The wire field shipped in 366692a1 was inert. MarkTracksMissing and
ClearTracksMissing are plain UPDATEs, and /api/library/sync is a
change-log feed: a row that never produces a change row is never
re-sent. Clients would have kept their stale copy until an unrelated
edit touched the track or the cursor fell out of the retention window
and forced a full resync -- so the flag existed and nothing ever told
anyone to read it.
Found by checking the consumer set rather than the code: the field was
threaded end to end and every test passed, because none of them asked
the question "how does this reach a client?".
Logged BEFORE the mutation, which is the opposite of the scanner's
log-after-success pattern, and deliberately so. The failure modes are
not symmetric. Log-then-fail-to-mark makes clients re-read a track that
has not changed: one wasted fetch. Mark-then-fail-to-log leaves the mark
with no change row -- and because both statements are idempotent
(missing_since IS NULL / IS NOT NULL guards), the next scan will not
retry the pair, so the client never learns. Permanently. A spurious
re-read is much the cheaper mistake.
Restoring logs too. A file coming back that nobody is told about stays
greyed out on every device until something unrelated touches it, which
would be a worse bug than the one being fixed.
Op is upsert, not delete: the track still exists and keeps its history.
Delete would tell clients to drop the row, which is precisely the design
#2704 rejected when it chose to ship state instead of filtering the feed.
Adds sync.LogChanges alongside LogChange, backed by an unnest batch
insert. Every existing caller mutates one entity, so per-row was right
for them; reconcile can mark a quarter of a library in one sweep, where
a loop would be thousands of round-trips inside an already-slow scan.
#2523 filtered missing tracks out of every path that CHOOSES music, but
the client sync feed was never touched: GetTracksByIDs has no filter and
the wire had no field for it. So every Android client held a cached
library containing tracks whose files are gone, with no way to tell, and
could queue them from any cache-first path -- the exact failure #2523
existed to prevent, reached by a different route.
Ships the state rather than filtering the feed, of the two options the
ticket weighed. A missing file is expected to come back: the scanner
clears the mark, and adopts the row if it returns renamed (#2528).
Withholding the row would mean a delete-and-recreate on every client for
what is usually a transient unmount, churning caches and throwing away
the identity #2528 works to preserve.
Room goes to v8. No hand-written migration: the pre-v1 destructive
fallback rebuilds from sync, which repopulates every row with the new
column -- exactly the case that policy exists for.
The interesting part was working out what "missing" means to a client,
and it is NOT "unplayable". Two findings shaped the fix:
Server search and album detail never filtered missing tracks either, and
that turns out to be right rather than an oversight. The consistent rule
the codebase already follows is that Minstrel never PICKS a missing
track for you -- recommendation, discover, mixes and browse all exclude
them -- but it does not hide one you went looking for by name or opened
an album to find. Hiding track 4 makes an album look wrong. So the fix
is to mark and to keep it out of queues, not to hide it.
And a track whose server file is missing still plays perfectly if its
audio is already in the device cache. ShuffleSource's offline pools
filter to exactly those residents, so it now clears the mark on the way
out: the bytes are local and the server's loss is irrelevant. Without
that, the queue filter below would have thrown away tracks that work,
turning a fix into an offline regression.
The queue protection is one choke point rather than five call sites.
setQueue is where playlists, album play-all, search, radio and cold-boot
resume all converge. dropUnavailable is pure so the index arithmetic is
pinned by tests -- removing entries ahead of the requested position
would otherwise start playback on the wrong track, and asking to start
on a missing track now starts the next playable one, which is the
"gets skipped" behaviour the operator asked for. An entirely missing
queue returns empty and the caller leaves the player alone rather than
replacing what is playing with silence.
CI caught a real bug, not just a brittle test. The page rendered an
attempt four hours away as "in 3h", because timeUntil floored the way
relativeTime does.
Flooring an elapsed time is honest: "3h ago" means at least three hours
have passed. Flooring a countdown is not -- 3h59m away became "in 3h",
so the operator comes back an hour early and finds nothing has happened.
It rounds now, with the boundary cases pinned: sub-minute is "any
moment", 59.6m is "in 1h", 24h is "in 1d".
That divergence is now the fourth documented difference between the two
formatters, all deliberate, all in snippet #2699 with a test asserting
they disagree so nobody unifies them later.
The test assertions were also genuinely wrong: they read raw textContent
from a template that wraps mid-sentence, so "last 2d ago" arrived as
"last\n 2d ago". Added a whitespace-normalising helper —
asserting on raw textContent makes a test fail when the markup reflows,
which says nothing about the behaviour.
Completes milestone #290. The sweeper has been running and the settings
have been editable, but the list itself said nothing about either, so
the only way to tell "not tried yet" from "asked twice and nothing came
back" was to go and read the Requests queue.
Each folder now carries its album's attempt record: how many times, when
last, when next -- or that it gave up, with the reassurance that a file
coming back and going missing later starts the process over. Null when
nothing has been attempted, which is the common case for a folder that
just went missing and would be noise on every row.
next_attempt_at is computed, not stored. The schedule is a function of
the attempt count and the current settings, so persisting it would go
stale the moment an operator edited the backoff -- and the card lets
them do exactly that.
Needed a forward-looking formatter. relativeTime deliberately collapses
a future timestamp to "just now" (pinned by its own test) because that
is the right answer for a clock-skewed past event; it is the wrong one
for a scheduled future attempt, which would have rendered "next just
now". timeUntil is its companion rather than a sign-aware rewrite: the
two read differently in the same sentence -- "last tried 3d ago, next in
4h" -- and a test asserts they disagree about the future on purpose, so
nobody later "fixes" the divergence.
The state lookup is one batched query for the whole page and best-effort:
this is context on a list whose real job is showing what is missing, so
a failure leaves the groups bare rather than failing the page. The
settings service is read with a nil guard falling back to the shipped
defaults, since contexts that wire routing without services exist and a
backoff projection is not worth a nil-pointer panic (rule #48).
Rule #27: the sweeper has been running since bab9b168 with no way to see
or change what it does. This is the half that makes it a feature.
Placed above the list it governs rather than under Integrations. An
operator looking at missing files is exactly the person deciding what
should happen to them; Lidarr is the mechanism, not the subject, and
separating the policy from the problem would mean finding one to
understand the other.
The card states the retry schedule the numbers add up to -- "6h -> 12h
-> 24h" -- because the fields are meaningless individually. "First retry
gap: 6" tells you nothing until you know it doubles and where it stops,
and an operator should not have to simulate the algorithm to predict it.
It recomputes as they type, including the clamp.
It also states the unnameable-album count with its reason. Those albums
will never produce a request no matter how long they sit in the list
below, because Lidarr cannot be asked for a release MusicBrainz cannot
name. Watching rows never move with no explanation is how a working
feature gets reported as broken.
Save errors surface the server's own message. The Go layer validates the
same ranges the CHECKs enforce and names the field, so the operator
reads "grace_hours must be 1-720" rather than a generic failure.
The dirty check compares only the stored fields: unnameable_albums is
server-computed, and including it would make the form look edited
whenever the library changed underneath.
Nine tests, including the schedule clamp, the disabled-until-dirty Save,
the surfaced validation message, and a failed load offering a retry
instead of an empty card. The existing missing-files page suite gains a
stub for the card's own settings fetch -- it mocks the whole admin API
module, so the card's imports would otherwise be undefined at mount.
go vet caught the Mount signature change: library_test.go calls it from
inside the package, so the earlier grep for "api.Mount(" missed it.
Rather than only appending the argument, the route table now includes
both admin surfaces from this arc. That test exists to prove every route
is actually registered — a 404 there means the route is missing — and
the two paths added today had no such coverage. Both are in the admin
group, so reaching the 401 is what proves they are wired.
The new service is passed as nil, matching the other optional services
in this call: the test asserts routing, never executes an admin handler,
and constructing a settings service would need a pool round-trip for
nothing.
GET/PUT /api/admin/library/reacquisition, so every knob the sweeper
reads is editable without a restart (rule #25). Routed under /library
beside the missing-files list it governs rather than under /lidarr:
Lidarr is the mechanism, but missing files are the problem the operator
came to solve, and that is the surface they meet it on.
The payload carries one thing the settings table doesn't: the count of
albums with missing files that can never be auto-requested, because
neither they nor their artist has an MBID. Nothing can be asked of
Lidarr for a release MusicBrainz cannot name, and a feature that
silently does nothing for part of its input reads as broken -- so the
card states the number instead of leaving it to be inferred. Counted
best-effort: the settings are the point of the endpoint, and failing the
whole card because a count query hiccuped would be the wrong trade.
Range errors come back as 400 naming the field. The Go-side validation
mirrors migration 0056's CHECKs precisely so the operator reads
"grace_hours must be 1-720" rather than a constraint-violation string
surfacing as a 500.
Answers the open fork on #2527's last slice: automatic, not a button.
Until now missing_since was a dead end -- reconcile marks it, every
selection path skips it, the admin surface lists it, and there it sits.
Two decisions carry most of the safety, both at the design level rather
than as rate limits bolted on afterwards.
The unit is the ALBUM, not the track. Lidarr acquires releases; there is
no meaningful "fetch me one track", and a track-kind request needs a
recording MBID plenty of files lack. Grouping means the loss that
produced #2523 -- three reorganised albums, ~40 missing files -- becomes
three requests instead of forty. The flood problem mostly dissolves.
And nothing is requested until a file has been missing longer than the
grace window (24h default). A filesystem lies transiently: an unmounted
volume, a container that started before its media mount attached, a NAS
mid-reboot. Every one of those resolves itself well inside a day at no
cost. missing_since is never re-stamped (#2523), so it is a true "gone
since" clock to measure against, not "when we last noticed". This is
the difference between automatic and trigger-happy.
Then the backoff proper: 6h -> 12h -> 24h -> 48h per album, clamped to a
week, three attempts before giving up, and a per-pass ceiling so a
genuinely large loss trickles instead of dumping hundreds of rows into
the queue. Giving up is stamped as a timestamp rather than inferred from
attempts >= max, so the verdict survives an operator later raising the
maximum and the surface can say when.
A sweeper, not a hook inside reconcile. Reconcile runs inside a scan and
has no business deciding to talk to a third-party service; it also
re-runs often, which would make "attempt once, then back off" awkward to
express. A worker paces itself, survives a restart, and retries without
needing another scan. Recovered albums have their state deleted rather
than reset -- a future loss is a new problem, not a continuation.
Requests are attributed to the oldest admin: lidarr_requests.user_id is
NOT NULL and a re-acquisition has no requesting human, so this keeps the
row auditable and in the same queue as everything else without inventing
a synthetic principal the schema would have to understand.
Auto-approve defaults ON. Requests are created pending and nothing
reaches Lidarr until approval, so with it off this would be a
notification rather than an attempt. Lidarr disabled leaves the request
pending rather than counting a failure -- the record of intent is still
right and becomes actionable the moment Lidarr is configured.
Albums with no MBID are counted, not silently skipped: nothing can be
asked of Lidarr for a release MusicBrainz cannot name, and quietly doing
nothing would read as the feature being broken.
Settings are DB-backed per rule #25 with CHECK-guarded ranges, validated
in Go as well so the API answers 400 rather than surfacing a constraint
violation. The admin card and the state on the missing-files page are
next; this is the engine.
Every comment naming a path in flutter_client/ now resolves to nothing,
which is the failure mode this project has already been bitten by twice
-- drift #572 came from delete.go describing behaviour it no longer had,
and that same docstring was still wrong when it was fixed last week. A
pointer to a deleted directory is the same thing in slower motion: the
reader follows it, finds nothing, and cannot tell whether the comment is
stale or they are looking in the wrong place.
Three treatments, per what each comment was actually doing:
- Naming a concept ("mirrors db.dart's CachedTracks Drift table"):
keep the concept, drop the path. The Drift table is why the entity
looks as it does; the file it lived in is not.
- Pure port bookkeeping ("Mirrors <path>." and nothing else): deleted.
Git history records the port; the comment only restated it.
- Substance introduced by a pointer (a lifecycle list, a 200 px/s
threshold, an inverted control-row placement): keep the substance,
drop the lead-in.
The two Go comments were the valuable ones and got more than a trim.
They stated a live contract -- "field names match the client's FromJson
helpers exactly, or fields are silently dropped" -- against a client
that no longer exists. They now name the real consumer,
SyncResponseWire.kt, and say why the failure is silent there too:
kotlinx.serialization skips unknown keys, so a renamed field arrives as
a default value rather than an error.
The ticket counted 64 files by grepping flutter_client/. A second tier
turned up during the sweep: 15 more references naming bare Dart files
(player_bar.dart, now_playing_screen.dart:464, auth_provider.dart) with
no directory prefix. Same dead tree, same treatment, folded in here.
Comments only -- verified no non-comment line is touched in the diff.
Superseded by the M8 native Android rewrite. Last touched 2026-05-31,
no workflow has built it since flutter.yml was removed, and rule #22
says a replaced path goes rather than lingering as something a reader
has to work out the status of. 245 files, ~24.6k lines.
Config references go with it: the .gitignore block (and its now-empty
"# Flutter" header), the .dockerignore entry, and renovate's ignorePaths
entry, which was suppressing dependency scanning for a directory that
no longer exists.
ci-requirements.md said ci-flutter "will retire once that directory
goes". It has gone, so the doc now says so -- CI-Runner can drop the
image, and nothing in this repo needs a Flutter toolchain.
One thing is kept rather than deleted: shared/fabledsword.tokens.json.
It lived under flutter_client/shared/ but was never Flutter's property
-- it is the canonical statement of the palette, the only place the
dark, light and flat cohorts are written down together, and
FabledSwordTokens.kt names it as its source of truth. Losing it would
have been collateral damage, so it moves to the repo root with a README
saying what it is and that neither client generates from it. That
comment in FabledSwordTokens.kt is repointed here.
What deliberately does NOT change: `runs-on: flutter-ci` in android.yml
and release.yml. That is a runner LABEL, not a path -- the Android jobs
schedule on it while pulling ci-android:36, per the label/image split
ci-requirements.md documents. Removing it would break scheduling for a
cosmetic win, so the doc now spells that out beside the retirement note.
Left for #2710: 64 files whose comments still name flutter_client/
paths. Sweeping them here would have buried the deletion, and each
needs a judgement -- keep the substance and drop the dead path, delete
pure "ported from" bookkeeping, or leave design rationale that happens
to mention the Flutter build.
Two findings, both fair:
LibraryScreen was one line over the 60-line cap once Genres and Years
were added to its pager. Split the page bodies into LibraryTabPage, so
the screen is the scaffold and tab bar while the routing table lives on
its own -- adding a tab is now one line there and one label in
LIBRARY_TABS, rather than growing a function that was already at its
limit.
The decade arithmetic used a bare 10 twice. Named it YEARS_PER_DECADE:
floor-to-decade reads as arbitrary without it.
#367 shipped genre and year browsing on web only, which left the web
tab bar's own comment -- "mirrors Android's LibraryScreen" -- half
aspirational. Android now has both, straight after Albums, in the same
order the web bar uses.
Server-backed, and that is the one real decision here. Every other
Library tab reads Room, and building these indexes locally was the
obvious move: the cache is a full mirror and carries both genre and
releaseDate. It does not work. /api/library/sync hydrates through
GetTracksByIDs, which has no missing_since filter, and neither
SyncTrackWire nor CachedTrackEntity has a field for it -- so the cache
holds tracks whose files are gone and cannot tell you which, while the
browse index excludes them. A locally-derived index would quietly
disagree with the server's and with the web client, and could offer a
genre that exists only in missing files. Filed as #2704; until it is
resolved these two tabs need a connection, and their empty states say
what they are rather than looking broken.
Genre is a query parameter end to end, never a path segment: "Rock/Pop"
is a real ID3 tag and a slash does not survive a path. That is also why
the drill-down is a second state inside the tab instead of a nav
destination -- a route would have had to carry the label.
Index shapes mirror web because the reasoning was already worked out
there: genres default to count order, since raw tags carry a long tail
of one-offs that A-Z buries the real genres under, with an A-Z chip for
when you already know the name; years group by decade, newest first,
because a flat list of every year in a decades-deep library is a wall
of numbers. Page size matches web's BROWSE_PAGE_SIZE so "Load more (N
left)" steps identically on both.
The orderings and grouping are pure functions, tested: server order left
alone under count sort, case-insensitive A-Z, no mutation of the loaded
state's list, a slashed tag surviving the filter intact, decade
bucketing including the boundary year, and a count label that stays
blank rather than flashing "0 albums" while the first page loads.
The 2026-08-16 diagnostics show a session that did not stutter so much
as end. In Doze the 1Hz poll freezes -- 14:22:53 and 14:26:14 report
byte-identical snapshots, and that flat 5000ms sonos-vs-local delta is
the last poll's staleness held still, not drift. When the screen came
back on, the first poll in 3.5 minutes found queue track 10 at 113s,
stopped. The Sonos had advanced, played 1:53 of a flac and quit while
the phone slept. The app then reported that faithfully and did nothing
about it, twice more, until the operator noticed.
A UPnP renderer streams autonomously, which is the whole point of
casting and also why a dead stream is invisible: pollOnce read STOPPED,
called applyTransportStopped and returned. Nothing asked "we meant to
be playing -- why aren't we?"
RemoteStallWatchdog asks. It is pure decision state -- no coroutines, no
SOAP -- so the counting, keying and giving-up is testable without a
renderer, and pollOnce just acts on the verdict.
Conservative by construction:
- STOPPED or an error status only. PAUSED is left alone: that is
somebody at the Sonos app or a wall controller, and taking the
transport back off a person is a fight they always lose. A stream
that dies stops, it does not pause.
- Only against play intent. A stop we asked for is not a stall.
- Three consecutive polls must agree. Sonos passes through STOPPED
between queue items, so one reading would make every track change
fight itself.
- Three attempts per track, 5s apart, then give up -- an unplayable
file must not become an infinite retry loop against a speaker.
- Resume seeks back to the last position seen while playing, so a
stream that died 90s in comes back near there, not at zero.
GetTransportInfo now keeps CurrentTransportStatus, which it previously
parsed and discarded. ERROR_OCCURRED is the only unambiguous way to
tell "the stream died" from "somebody pressed stop", since both land in
STOPPED. Absent or unrecognised reads as OK so a quiet renderer is
never mistaken for a broken one.
Giving up reports kind="stalled" through the existing
PlaybackErrorReporter: snackbar for the user, admin-inbox row for the
operator. That kind has been in migration 0032's CHECK whitelist and
labelled on the admin page since the table was built, and nothing had
ever emitted it.
This does not explain WHY the stream died -- see #2700 for the
hairpin-routing lead. It does mean a dropout is a recoverable hiccup
instead of the end of the session.
The tab list is pinned by name and order, so adding Missing files
failed the assertion. That is the test doing its job: the nav is a
deliberate ordering, not an accident, and a new entry should have to
be declared rather than slipping in.
Renders GET /api/admin/library/missing under Admin -> Missing files.
Folder-grouped, because that is the unit an operator decides about: the
case behind #2523 was three reorganised albums, and forty individual
rows hides that it is really three decisions.
Each row leads with the fact that settles whether a missing file is
worth chasing -- "last played 2d ago" against "never played". The group
header carries how many tracks and how long they have been gone.
Read-only. No remove button anywhere: the row, its play history and its
likes survive a file going missing, and the scanner clears the mark by
itself when the file returns (or adopts the row if it returns renamed,
#2528). The page says so in its own copy rather than leaving the
operator to infer it.
Empty state explains the feature instead of the emptiness -- what puts a
row here (moved outside Minstrel, deleted, a drive that didn't mount)
and that rows leave on their own. Someone who has never seen this page
should not have to guess.
Paging follows the house pattern -- plain offset into the factory,
wrapped in $derived so a page change re-creates the query with a new
key. Passing a getter instead would capture the key once and paging
would silently not refetch. The pager only renders when it can do
something.
Admin quarantine, admin playback-errors and library/hidden each carried
a byte-identical private copy of the same coarse "3d ago / 5h ago /
12m ago / just now" formatter. Writing the missing-files surface would
have made it four, so extract it instead.
They are one concept, not three that happen to look alike: each shows
the age of something an operator is deciding about, and they have to
agree -- a row reading "2d ago" on one screen and "2 days" on another
makes the reader wonder whether the two mean different things.
Three near neighbours are deliberately NOT folded in, because they are
different intents rather than drifted copies:
- HistoryRow shows a weekday and clock time under a week ("Tue 21:40"):
for listening history, WHEN you played something beats how long ago.
- ActiveSessions.when() writes prose ("1 hour ago", "yesterday") and
falls back to a locale date past 30 days -- a security surface where
the longer form reads better.
- PlaylistCard.refreshedLabel() is day-boundary aware and prefixed
("Refreshed today"), and already carries a comment saying it is
deliberately not the m/h-ago style.
Merging any of those would mean forcing one caller's wording onto
another, which is the wrong-abstraction failure, so they stay put.
Tests pin the boundaries the copies never covered: each unit step, that
only the largest whole unit is reported (25h is "1d ago", never
"1d 1h ago"), and that a future timestamp from a skewed client clock
degrades to "just now" instead of rendering a negative age.
Android was already skipping these by accident: toPlayableTrackRefs
filters on a non-empty streamUrl, and the server stopped emitting one
for a missing file, so they never reached the queue. Correct behaviour,
no idea why -- the row just sat there greyed with the same treatment as
a track deleted from the library, which is a different and permanent
thing.
isAvailable now covers both cases explicitly rather than inferring one
from an empty URL, so every reader (row alpha, click gating, queue
building) gets the same answer from one place. The flag stands on its
own deliberately: a detail fetched before the file went missing can
still carry a stale streamUrl from cache, and that must not resurrect
the row.
The row says which kind of dead it is. A missing file gets "· File
missing" on the subtitle line, because that one can fix itself -- the
scanner clears the mark when the file returns and adopts the row if it
returns renamed (#2528) -- so it is worth telling the user about. A
removed track keeps its bare greyed treatment; there is nothing to act
on once it's gone from the library.
Matches the web treatment landed in 4c49ee2c (rules #23/#27: parity,
not web-only).
The row treatment for a dead playlist entry already existed -- muted
text, no play on click, no drag, no kebab, never "now playing" -- but it
only fired for track_id === null, the track-deleted case. A missing file
kept a live-looking row that failed on click.
The behavioural gate now covers both, and the presentation distinguishes
them, because they mean different things to the person reading the list.
A removed track is gone for good and keeps the strikethrough. A missing
file is a track we still have -- history, likes, the lot -- whose bytes
aren't on disk right now, so it gets an explicit "File missing" and a
title explaining it stays in the playlist and comes back on its own if
the file does. A strikethrough there would claim it was deleted, which
is a lie about a file the scanner may well adopt back tomorrow (#2528).
Skipping routes through playlistTrackToRef, which already returned null
for removed tracks and whose callers already filter nulls. Adding the
unavailable check there means every queue builder -- PlaylistCard,
systemRefetch, the detail page -- skips a missing file without any of
them learning what missing_since is.
Remove stays available on a dead row: the owner must still be able to
take it out of their own list.
The scan has marked missing files since f6d1cf24 and every selection
path filters them out, so they cause no harm -- and are invisible. The
operator found out about the first batch only because an unrelated MBID
backfill logged "no such file or directory" forty times.
GET /api/admin/library/missing reports them, grouped by directory. The
grouping is the whole ergonomic argument: the case that produced #2523
was three reorganised albums, which a flat list renders as forty
unrelated problems and a folder list renders as three decisions.
ListMissingTracks orders by directory so the handler can fold runs
without a map, which also keeps the query's ordering instead of Go's
random map iteration.
Each row carries last_played_at, nullable, because "gone six months,
never played" and "gone yesterday, played 200 times" deserve opposite
reactions and a file path tells you neither. The correlated MAX needs
its ::timestamptz cast or sqlc infers interface{} and the Go layer
loses the type.
Read-only, deliberately. Nothing here deletes: a missing file keeps its
row, its play history and its likes because it may come back, and if it
comes back renamed the scanner adopts it (#2528). The route sits under
/library rather than /tracks so it can't be confused with the
destructive DELETE /admin/tracks/{id} beside it.
Every browse, discover and mix query filters missing_since, so a track
whose file vanished disappears from the places Minstrel chooses music.
A playlist is different: the entry is there because the user put it
there, and silently dropping it rewrites their list behind their back.
So playlists keep the row and mark it instead. ListPlaylistTracks now
carries missing_since (still deliberately unfiltered), the service
layer surfaces it as PlaylistTrack.Unavailable, and the wire gains
"unavailable" on each entry.
A missing entry also loses its stream_url. Refusing to hand out a URL
that cannot serve is stronger than trusting every client to honour the
flag, and "stream_url": null is a shape the clients already model --
PlaylistWire.streamUrl is documented nullable for the track-removed
case -- so an older build degrades to "present but not playable" with
no change.
Nothing is deleted here and nothing should be: the row, its play
history, its likes and its taste contribution all survive a file going
missing, because the file may come back (and #2528 will adopt it if it
comes back renamed).
Also corrects two comments that had drifted into lying. delete.go still
claimed the file-gone case was NOT auto-reconciled and told admins to
delete rows by hand -- untrue since f6d1cf24, and that exact staleness
is what produced drift #572. It now says what DeleteTrackFile really is:
the destructive admin action, which CASCADEs play_events and likes, and
is emphatically not the missing-file path. watcher.go claimed the
safety-net scan "covers anything missed"; the walk only covers
additions, and it is reconcile that covers removals.
The shell is a Column (content weight(1f), then the bar), so the
content viewport already ends at the MiniPlayer's top edge. But
nothing owned the bottom navigation-bar inset under edge-to-edge:
each in-shell screen's own Scaffold claimed it via the default
contentWindowInsets and padded its content up by the nav-bar height
a second time. That padding is the dead strip the operator sees
between the last list row and the bar — and the bar's own bottom
was drawing under the gesture pill.
ShellScaffold now owns the inset end to end: the content region
consumes it, and a Spacer below the MiniPlayer re-holds the space
for the system bar (unconditional — MiniPlayer renders nothing when
no track is loaded). Modifier.consumeWindowInsets alone can't fix
it: ScaffoldLayout reads contentWindowInsets.asPaddingValues()
directly, outside the modifier consumption chain, so every in-shell
Scaffold is handed the new zero ShellContentWindowInsets. The
full-screen routes (NowPlaying / Queue / Login / ServerUrl) keep the
default — no shell sits above them.
Also drops the hardcoded 140dp bottom contentPadding on Album and
Playlist detail, a Flutter-era value for a player bar that overlaid
its list; here the shell reserves that space in layout already.
Operator decision: keep the ID3v1 table canonical, fix the casing.
The operator's library carries "Edm", "Idm", "Aor", "Uk Garage", "Uk
Hardcore", "Trap Edm", "Glitch Hop Edm" and "Children'S Music" — an external
tag editor title-cased the whole genre field. The "'S" is the giveaway.
Fixed at SCAN time, not in the display layer: taste_profile.sql reads
tracks.genre directly, so a cosmetic-only fix would leave the taste
vocabulary holding "Edm" while the UI showed "EDM", and any correctly
tagged file would contribute a second, separate tag.
trueUpCasing only ever changes case, never letters, so it cannot silently
turn one genre into a different one — that is what separates it from the
label-remapping idea this task rejected. Two narrow rules:
- A short, evidence-led acronym list, matched case-insensitively so "edm",
"Edm" and "EDM" all land on "EDM". This is a deliberate exception to the
project's rule that genre case is exposed as the file says it: "Rock" and
"rock" still stay separate rows, because folding those is a judgement about
labels, whereas there is no genre named "Edm".
- Apostrophe suffixes from a FIXED contraction list, so "Children'S" is
repaired while "O'Brien" and "D'Angelo" keep their capital. A blanket
"lowercase after an apostrophe" would have broken both.
Matching uses the word's letter core rather than the raw word, so "(Edm)"
and "Edm," are repaired and their punctuation re-attached. Interior
punctuation stays in the core, so "Lo-Fi" and "R&B" are compared whole and
cannot match a fragment by accident. My first version missed this and a test
expecting "(Live EDM)" caught it.
Names resolved from the ID3v1 table are deliberately NOT re-cased, per the
operator's call — entry 40's "AlternRock" stays as the table spells it, with
a test pinning that so a later tidy-up doesn't quietly "fix" it.
tagReadVersion 1 -> 2, so this reaches the existing library on the next scan
rather than new files only. That re-read reuses stored durations, so it costs
tag reads and no ffprobe.
Operator decision: the taxonomy this task proposed is cancelled. With their
repaired library measured — 391 genres, 90,774 tag applications over ~24,185
tracks, so ~3.7 genres per track — multi-membership already puts each track
under everything it claims, and grouping would add nothing while destroying
real specificity (Neurofunk, Wassoulou, Soukous are not noise).
The task's premise was also wrong. It argued from case variants, "Alt. Rock"
abbreviations and a junk tail; none exist. That apparent mess was the scanner
welding multi-value tags (#2499) plus ghost rows from deleted files (#2523),
both ours, both now fixed. A category system would have papered over both.
So the ask reduces to sorting and search. The search box already existed
(QuickFilter, with its own no-matches state), so this adds only the sort:
count-first by default — the server's order, and the right default since the
head is where you're going — or A–Z for when you can already name the thing
but can't find it among 391 rows. The filtered count now reads "12 of 391"
so a filter's effect is visible.
Client-side only: /api/library/genres is unpaged and already returns the whole
set (~12KB), so neither control needs a round trip or a server change.
Two things the tests pin down:
- The sort COPIES before sorting. With no filter applied the derived list is
the very array held by the query cache, and Array.sort mutates in place —
sorting it directly would reorder cached data under every other consumer.
- Count mode passes the server's order through rather than re-sorting. The
fixture is deliberately not in count order so the test asserts pass-through
instead of coincidence.
Verified locally: svelte-check 0 errors, 110 files / 788 tests.
Auditing the gating turned up two problems.
verify-release only checked the APK. Because it runs with `always()`, it
runs even when image-release FAILED — so android succeeding while the image
push died would have reported "verified" on a release with no immutable
:vYYYY.MM.DD image. That is exactly half of what was missing when
v2026.08.07 had to be re-cut, so the guard would have caught the incident we
had and waved through its mirror image. Now checks the image too, via
docker manifest inspect.
"Attach APK to gitea Release" resolves the release by tag and fails if it is
absent — but it is the LAST step, so a bare `git push origin vX` built an
APK for several minutes before discovering it had nowhere to put it. Same
check now runs immediately after version computation: seconds, not minutes.
Releases created through the API create tag and release together and pass it.
The rest of the gating audits clean, and one part is worth not "fixing":
image-release's `if: !failure() && !cancelled()` looks odd next to
`needs: [android-release]` but is correct. On main pushes android-release is
SKIPPED, and a skipped dependency is not success() — so the obvious
`if: success()` would silently stop main from ever publishing :latest.
Steps 4/5 vs 6 are mutually exclusive on the tag context, and every image
step gates on the Dockerfile+go.mod guard.
Validated: YAML parses, and `bash -n` over every run: block in all three
jobs is clean.
v2026.08.07 had to be re-cut, and the tag build's android-release job
never started — no job log was written at all, so all eight steps reported
`failure` with none executed and image-release showed `skipped`.
The run was red, but the release PAGE rendered fine and main's own push
build had already moved :latest, so the code was deployable and nothing
looked obviously wrong. What was actually missing — the attached APK and
the immutable :vYYYY.MM.DD image — is easy to skim past, and I nearly did.
This cannot prevent that. The cause was a runner failing to launch a
container, not anything in this file, and it did not reproduce on an
unchanged re-run. What this does is make the CONSEQUENCE legible: an
incomplete release now fails with a named error instead of eight mystery
step failures, and the message says to re-run the run rather than delete
and re-create the tag.
`if: always()` is load-bearing — the job has to report precisely when the
jobs above did not succeed.
Correcting the record while here: I first blamed this on the workflow's
`cancel-in-progress` concurrency block. That was wrong. Cancellation needs
a NEWER run in the same group, and there was exactly one run on the tag
ref (total_count 632 -> 633 on release creation); the main-push runs sit
in a different group. Plausible mechanism, unchecked precondition.
Validated locally: YAML parses, `bash -n` clean, and the asset-parsing
logic unit-checked against a release with an APK, one with no assets, and
one with a non-APK asset.
Third attempt at the same assertion, so I stopped guessing and got vitest
running locally instead: the web lane uses the same ci-go image, so
`docker run ... -w /src/web ci-go:1.26 npx vitest run` works and turns a
5-minute CI round trip into 7 seconds.
/^Skip/ matched the "Skip rate by week" sparkline column as well as
"Skip (last wk)", just as /Plays/ had matched the caption. Exact names
say what the assertion means and cannot drift onto a neighbour.
Verified locally before pushing: svelte-check 0 errors, 110 files /
786 tests pass.
getByText(/Plays/) matched my own new caption as well as the header, since
the caption explains which columns cover the window. Query by columnheader
role instead, which is what the assertion actually means.
Also reordered the caption: prepending the clarification turned it into a
run-on that opened mid-explanation before saying what the chart was.
Two CI failures, both in my own new tests, both informative.
settings: queryByText(/≈/) matched the LEGEND explaining the glyph rather
than a delta, so the "no delta" case failed on the explanation being
present. Delta spans now carry data-testid so a test can name what it
means instead of pattern-matching prose that sits next to it.
tuning: getByText("40%") found two elements. testing-library matches an
element and its OWN direct text nodes, so the skip cell still matches
"40%" despite the trailing play-count span — and discover late-week
completion is also 40%. Genuinely ambiguous now; assert the count.
The metrics card had one volume threshold doing two jobs.
recMetricsLowVolume = 20 is a DISPLAY floor — below that a skip rate is
anecdote — but the card then presented deltas as though it were also a
DECISION floor. Those differ by an order of magnitude: detecting the ~13pp
differences that matter needs ~133 plays per arm for 80% power at a=0.05.
So Discover's taste-matched (59 plays) and random-unheard (70) both rendered
as full-confidence rows with a bold delta beside them, and that comparison
sits at p ~ 0.06. The card said "signal"; the arithmetic said "maybe". It
produced a recommendation the data didn't support, and any reader with the
same numbers would have made the same call.
Deltas now carry a 95% margin of error and a `distinguishable` flag, computed
server-side so both clients read the same arithmetic instead of each
re-deriving it. Skip rate is a two-proportion difference; completion is
Welch, which needs a variance — hence completion_sqsum in the query. It is
the sum of squares rather than stddev_samp on purpose: raw source rows are
merged into surface families in Go, and sums of squares combine across groups
exactly whereas standard deviations cannot.
recMetricsLowVolume is untouched. "Too thin to show" and "too thin to act on"
are different questions.
Web renders an indistinguishable delta as dimmed and prefixed "≈", with the
range on hover and a legend explaining the glyph. Colour is withheld unless
the delta clears its margin — colouring noise red is what made the old card
misleading. Breakdown rows go through the same path; those are the thinnest
samples on screen and where the old card misled most.
Also fixes the admin trends view, which had the same problem worse: its
"Latest skip"/"Latest completion" columns are one WEEK while the adjacent
Plays column is the whole window. I misread exactly that and briefly
concluded Deep cuts was the worst surface, from ~17 plays in a single week —
over 180 days it is one of the best. Headers now name their period and the
skip cell carries that week's play count.
#2524: resolveArtist now recognises a duplicate-MBID unique violation as the
expected condition it is, matching resolveAlbum. Two rows mapping to one
MusicBrainz artist is a merge candidate, not a fault; without the branch it
logged a generic warning plus a Postgres ERROR line on every scan, which
teaches an operator to ignore database errors.
Track identity was file_path, so a file that came back renamed or in a
different directory looked like a deletion plus an unrelated new track:
the old row kept the like and every play_event while a fresh zero-history
row appeared, and nothing connected them. A liked song read as unliked, its
play count reset, and Rediscover could offer it as a discovery — silently.
Renumbering an album was enough, which is what happened to the operator's
copy of Minutes to Midnight.
Adoption re-points the existing row's file_path at the new location and
clears its missing mark. The normal UpsertTrack then conflicts on file_path
and updates THAT row, so the track id survives and likes, plays and
playlist memberships travel with it — and clients see an update rather than
a delete-and-create, so no cache churn either.
Matching is MBID first (identifies the recording, so it survives a
re-encode), then file_size + duration_ms for untagged files. Both
fingerprint components must be non-zero: duration_ms is 0 when ffprobe
failed, and matching 0 against 0 would pair up unrelated broken files.
Only rows already marked missing are eligible — a row whose file is present
elsewhere is a duplicate, not a move, and re-pointing it would corrupt the
copy that still exists. An ambiguous match inserts fresh rather than
adopting one arbitrarily: a fork is recoverable later, a wrong merge isn't.
Scan is now three phases, and the order is the point. Adoption can only
claim a row that is ALREADY marked missing, but reconcile previously ran
after processing — so a rename performed while the server was down surfaced
the deletion and the addition in the same scan, the new path inserted first,
and the fork became permanent. Enumeration is therefore separated from
processing so reconcile can run between them: walk (paths only, no tag
reads or probes) -> reconcile -> process in walk order.
Consequence worth knowing: when reconcile refuses (an absent root, or a
reorganisation exceeding the 25% mark cap) adoption cannot fire and renamed
files fork as before. That's the pre-#2528 behaviour rather than a new
failure, and the warning now names it.
The old outer walk-error branch was unreachable — the callback always
returned nil, so WalkDir never surfaced an error — and verifyRootsPresent is
the real protection, so enumerate counts walk errors instead of pretending
to abort on them.
Nothing in Minstrel ever noticed a deleted file. The walk only visits
paths that exist, so a row whose file was gone was never scanned, never
errored, never counted — permanently invisible. classifyEvent ignores
fsnotify removals by design, and the safety-net scan is the same walk, so
it covers additions only. Rows accumulated forever.
Found on the operator's library: a completed scan reported
skipped=24185 errored=0 while the MBID backfill (which opens files by DB
path rather than walking) logged ~40 "no such file or directory" across
three reorganised albums. Those rows also kept their pre-#2499 welded
genre, which is how this surfaced — the version-stamped tag re-read can
only reach files the walk visits.
The harm is not cosmetic. tracks is the candidate universe for
recommendation.sql / discover.sql / system_mixes.sql and nothing filtered
on file existence, so a mix could spend a slot on a track that cannot
stream.
Marks rather than deletes. A missing file is a claim about the filesystem
and the filesystem lies transiently — an unmounted volume, a network
blip, a container that started before its media mount attached. Every
sweep in internal/gc resolves a truth INSIDE the database and is safe to
run blind; this one is not, so no deletion happens here. Three guards
refuse to act on ambiguous evidence: every scan root must resolve to a
non-empty directory, the walk must have seen at least one file, and one
reconcile may newly mark at most 25% of the library. Clearing a mark is
never the dangerous direction, so it runs unconditionally — otherwise a
library that tripped the cap could never recover once the mount returned.
Only a full Scan reconciles. The walk's set of seen paths is the
evidence, and ScanFiles has no basis for concluding anything about files
it did not look at.
Excludes marked tracks from all 13 track-emitting queries (radio x2,
system mixes x5, discover x4, most-played x2), the 6 play-history seed
picks, and the genre browse axis. Deliberately NOT filtered: the shared
ListPlaylistTracks read path, because it also serves user-curated
playlists where hiding a track the user added would be wrong — system
playlists shed orphans on their next daily rebuild instead. History and
the taste profile also keep them: those record the past, and a track you
played 200 times still says something about your taste.
Reconcile tallies land in scan_runs so a disappearance is visible rather
than discovered when a mix comes up short.
dhowden/tag's readTFrame splits ID3v2 null-separated multi-value text
frames and rejoins them with the EMPTY string, so a file tagged
"Alternative Rock" + "Rock" was stored as "Alternative RockRock". It also
leaves bare numeric ID3v1 references unresolved, which is why the
library showed genres like "4017" and "526617".
This corrupted more than the browse axis added in #367: taste_profile.sql
reads tracks.genre directly, so the welded tokens were entering the taste
profile's tag vocabulary, and recommendation.sql/discover.sql were
comparing them as single opaque tags. Genre counts were wrong everywhere.
ffprobe is not a fix — ffmpeg's read_ttag calls decode_str once with no
loop, keeping only the first value. Truncating multi-genre tags would
blunt the similarity signal genre mainly feeds. So the TCON frame is now
parsed directly (ID3v2.2/2.3/2.4, all four text encodings, per-frame and
tag-level unsynchronisation, numeric and parenthesised ID3v1 references);
everything else still comes from dhowden/tag. Values are stored
";"-delimited, which the read side already splits on, so no query changes.
Existing rows are repaired without an operator-run rebuild: migration
0054 adds tracks.tag_read_version DEFAULT 0, below the scanner's current
tagReadVersion, so the next scan re-reads tags it would otherwise skip on
mtime. Such a re-read reuses the stored duration instead of re-running
ffprobe, keeping a repair pass tag-read-bound rather than one fork+exec
per file. Bumping the constant is how a future extraction fix reaches an
existing library.
Only ID3v2 is in scope — dhowden welds nowhere else. The Vorbis/MP4
repeated-field question is #2500, unproven and deliberately not built.
Two changes so a network handoff stops making the app refuse to play music.
## Correction first: half of what I proposed already existed
I recommended "require corroboration before ServerDown, since Unstable is
non-gating." ReachabilityMachine has done exactly that since it was written —
onProbeFailure takes Reachable → Unstable, and escalates only on corroboration
or the 120s backstop. There is even a test named `single probe failure is
unstable not down`. I proposed building a thing that shipped months ago.
Reading the machine properly turned up the real gap, which is narrower and more
specific.
## 1. Probe when the app returns to the foreground
The genuine missing piece, and #1209's own note had it backwards: it listed
this as "already happens via link probe." It doesn't. `recheck()` had exactly
two callers — a button in VersionTooOldBanner and pull-to-refresh — and nothing
observed ProcessLifecycleOwner. The link probe fires on a connectivity
*change*, so an app backgrounded on stable Wi-Fi gets none.
That made a stale ServerDown outlive its cause: the poll loop's delay() is
throttled while screen-off/doze, so recovery waited for whenever the OS next
let the loop run. June's capture recovering at "EXACTLY 22:31:10 app_foreground"
was the throttled delay resuming, not a deliberate probe — same timestamp,
different mechanism, and that difference is the whole bug.
NetworkStatusController now implements DefaultLifecycleObserver and calls the
existing recheck() on ON_START. force = true, so it also bypasses
ARBITRATE_MIN_GAP_MS: a user opening the app is exactly when a stale banner and
a refused track are most visible, and it's once per foreground.
## 2. A burst of op failures no longer corroborates itself
The actual defect in the escalation path. Corroboration required 2 op failures
within 30s — but a link handoff fails every in-flight request at once, so a
burst is ONE event producing N failures, not N independent observations that
the server is gone. Two simultaneous failures walked straight to Unreachable.
onOpFailure now drops a failure landing within CORROBORATION_MIN_SPACING_MS
(3s) of the last recorded one. Above the sub-second window a handoff occupies,
low enough that a real outage still corroborates within seconds once anything
retries.
## Why this matters more than the task implied
#1209 called the follow-ups "cosmetic in the diagnostics". They aren't.
OfflineGatedDataSource.gateOnHealth() throws OfflineException on ServerDown
BEFORE touching the network, and TrackRow disables rows. So a spurious
ServerDown means the app declines to play uncached tracks that would play
fine — for a blip that already resolved. The note's "captured skips advanced
fine" was timing luck, not evidence the gate is harmless.
## Tests
`two op failures plus a failed probe escalate immediately` used timestamps
500ms apart, which the new rule treats as a burst — so I re-spaced it and
renamed it `two SPACED op failures...`. That's a deliberate reversal of an
encoded expectation, not a broken test being patched.
Also re-spaced `stale op failures do not corroborate` (used 0 and 1_000): left
alone it would still have passed, but for the wrong reason — burst-dropping
rather than staleness — and a test that can't fail for its stated reason is
worse than no test.
Added: a burst of four failures plus a failed probe stays Unstable, and a burst
that never recovers still escalates via the sustained backstop, so dropping
duplicates can't make a real outage undetectable.
The foreground hook itself is unverifiable in a JVM test (ProcessLifecycleOwner
needs the framework, and there's no instrumentation lane). Checked instead that
nothing constructs NetworkStatusController outside Hilt, so init's
ProcessLifecycleOwner.get() only runs on the main thread during
Application.onCreate — the same pattern LiveEventsDispatcher already uses.
Last bullet of #367. From an album you like, one click to everything else from
that year or in that genre.
Year was free — AlbumRef already carried it. Genre was not: AlbumDetail is
AlbumRef + tracks and neither carried genre, because genre lives on TRACKS. So
both detail responses gained a derived `genres` array, computed from the
entity's tracks rather than stored, since an album's tracks can legitimately
disagree about genre.
Split and trimmed identically to the browse index. That's the invariant this
whole task turned on: if the chip's matching diverged from the index's
splitting, a chip would lead to a page that doesn't contain the album you
clicked from.
No year link on artist detail. An artist spans many years, so a single one
would be a lie about the discography — genres only there.
Genre lookup failure is logged and degrades to no chips rather than failing the
request; a navigation nicety must not 404 a detail page that otherwise loaded.
`genres` is always an array at JSON, never null, matching how every other list
field in this package is emitted.
## Type widening, and the TypeScript version of a lesson from earlier today
Adding a required field to AlbumDetail/ArtistDetail breaks every typed fixture
that constructs one. Six of them across three test files. That's the same shape
as the Go signature changes that cost three CI rounds in #2453 — change a type,
then go find everything that builds it — so I searched for the constructions
before pushing instead of after. All six updated.
Tests: the encoded href for a slash-bearing genre ("Rock/Pop" →
?g=Rock%2FPop), the year href, and the no-tags case rendering no chips at all.
gofmt verified clean via docker rather than guessed.
Client half of #367. Two new Library tabs, each an index plus a drill-down.
Genres are ordered by track count rather than alphabetically. Raw ID3 carries a
long tail of one-off tags, so alphabetical would bury the handful of genres you
actually have a library's worth of. Years are grouped into decades — a flat
list of every year in a decades-deep library is a wall of numbers, and the
decade is how people actually think about it.
## Selection travels in the query string, not the path
`?g=Rock%2FPop`, not `/library/genres/Rock%2FPop`. A slash-bearing genre cannot
survive a path segment — the server sees two segments, and a hard reload
wouldn't reconstruct it through the SPA fallback either. There's a test pinning
the encoded href and another pinning that the DECODED value reaches the API.
## Why these two pages don't use svelte-query for their lists
The indexes do — fetched once per mount, so static options suffice and the
cache survives bouncing in and out of a drill-down.
The drill-down lists deliberately don't. Their selection comes from the URL and
changes WITHOUT remounting the page, and this codebase has no
reactive-query-options pattern anywhere; inventing one here would be a larger
change than the feature justifies, and one I can't exercise locally. So they
use $effect keyed on the derived selection with an explicit Load more.
The stale-response guard is a plain `let`, not $state, and that's load-bearing:
as reactive state, reading the token inside the fetch path would make the
effect depend on its own writes. Its job is to discard a late response for a
previously selected genre instead of painting it over the current one.
## Also
Added the year filter to /library/albums' contract but NOT to that page's UI —
its infinite scroll is a svelte-query infinite query, and making it react to a
filter is the same reactive-options problem. The dedicated pages cover the
capability, which is the shape the task offered as its alternative.
Library tab bar's comment claims it mirrors Android's LibraryScreen. These two
tabs have no Android equivalent, so I noted that inline rather than leaving the
claim quietly false. Parity remains an open call.
Not yet done from #367's bullet list: genre/year quick-jump links on album and
artist detail. Year is free (AlbumRef already carries it) but genre is exposed
nowhere client-side — AlbumDetail is AlbumRef + tracks, and neither carries
genre — so it needs a small API addition. Following as its own commit.
One space. `name:` had to align with `query:` inside a composite literal where
both sat on their own lines.
Found via `docker run golang:1.25-alpine gofmt -l`, which is the actual point
of this commit: gofmt is available here the same way sqlc is, and there is no
reason to have let CI discover a formatting nit. Whole tree verified clean, not
just this file.
The substance of 1126bfcf was already sound — verify-generate, vet and the full
integration suite passed, so the genre-splitting behaviour holds against a real
database. Only the formatter objected.
Server half of #367. Web UI follows.
Genres are exposed AS-IS per the operator: split on the delimiter, trimmed,
but no case folding and no synonym mapping. So "Rock" and "rock" appear as
separate rows, as does "Rock/Pop" alongside "Rock" and "Pop". The raw spread
has to be visible before anyone can judge whether it needs normalising, and
the alternative is a mapping table to invent and then maintain.
Trimming is not an exception to that. Splitting "Rock; Pop" yields " Pop", and
showing that as a genre distinct from "Pop" would be a bug in OUR splitting,
not fidelity to the operator's tags.
## The correctness trap this had to avoid
ListAlbumsByGenre compared tracks.genre verbatim, while recommendation.sql and
discover.sql have always split it on [;,]. Building the browse index by
splitting while matching exactly would have listed genres whose pages are
empty — every multi-genre track unreachable from either of its genres.
So ListAlbumsByGenre now splits too. That also fixes Subsonic
getAlbumList?type=byGenre, its only caller, which silently missed every
multi-genre track. Its Genre param went *string → string as a result.
EXISTS rather than JOIN + DISTINCT ON throughout: the lateral split emits one
row per (track, fragment), so a join multiplies rows per album and needs
DISTINCT to undo itself. EXISTS asks the question directly, and the count
query then matches the list query by construction rather than by coincidence.
## Genre is a query parameter, not a path segment
Because "Rock/Pop" is a real ID3 tag — the one the task itself cites — and a
slash cannot survive a path segment: Go normalises %2F and the router would
split the value in two. So filtering rides GET /api/library/albums?genre=,
which also reuses the existing paged album surface instead of adding a
parallel one.
Endpoints:
GET /api/library/genres unpaged index + track counts
GET /api/library/years unpaged index + album counts
GET /api/library/albums?genre= filtered page
GET /api/library/albums?year_from=&year_to= filtered page, either edge open
The indexes are unpaged deliberately: a client needs the whole set to render a
browsable picker, and paging would let it show only a prefix of an ordering
the user didn't choose.
Two refusals rather than guesses: genre+year together is a 400 (the UI browses
them as separate axes, and quietly dropping half a filter would report a
narrower result than it returned), and an inverted year range is a 400 rather
than being silently swapped.
Undated albums are absent from the year axis rather than bucketed under 0 —
"unknown" is not a year, and a 0 row would sort to one end of a chronological
list looking like data.
Tests: parseYearFilter is pure and runs in the fast lane. The integration
tests assert the thing that would otherwise be silently broken — that a
"Rock;Pop" track is reachable from BOTH genres, that "Rock/Pop" survives as a
filter value, that fragment whitespace is trimmed, and that undated albums
stay out of every year range. Reused the existing seedAlbum/seedTrackWithGenre
fixtures, which already took exactly the year and genre arguments needed.
Closes the disagreement left open by #2453: requestlog.go logged raw
r.RemoteAddr while the Active-sessions surface resolved through the operator's
configured proxy depth. Behind a proxy — the normal deployment for anything
public — every access-log line carried the same useless proxy address, and the
two surfaces contradicted each other about who connected. Logs and UI
disagreeing is worse than either being wrong alone, because it costs you trust
in both.
`remote` now holds auth.ClientIP(r, hops). The attribute KEY is deliberately
unchanged so existing log greps keep working; only its accuracy improved.
Wiring note. The access log covers /healthz and the SPA, so it's registered
before the pool-bearing branch that used to build the settings service. Rather
than close over a variable reassigned later — which works, but leaves a
mutable-after-registration seam and an awkward question about races — I hoisted
netsettings.New above the router entirely. It already handles a nil pool by
returning a default-valued service, so no branch is needed and the accessor
stays a plain method value.
Applied the lesson from the last three CI failures BEFORE pushing this time: a
bare-identifier grep for `requestLog(` found three call sites in
requestlog_test.go that a qualified pattern could never have matched, since
the function is package-private and its tests are in-package. Also swept
netsettings.New and ClientIP the same way.
Tests: the behaviour change gets its own table — nil accessor and depth 0 log
the socket peer, depth 1 through a PUBLIC-addressed proxy logs the client
(the exact case the old heuristic got wrong forever), depth 2 reaches through
a CDN. Added `remote` to the required-keys assertion so the attribute can't
quietly disappear.
TestSetHops_RejectsOutOfRange caught a real ordering bug in code I wrote in
the same commit: the nil-pool guard sat ahead of the range check, so
SetHops(-1) on a service with no pool returned "network settings unavailable"
instead of ErrHopsOutOfRange.
Range first is correct, and the distinction is user-visible rather than
cosmetic: the argument is invalid regardless of whether the database is
reachable, and admin_network.go maps ErrHopsOutOfRange to 400 while anything
else becomes 500. The old order blamed the server for the caller's input.
Note this is the first failure in this sequence that wasn't a missed call
site — vet and golangci-lint both passed, and a test asserting a specific
sentinel error found it. Worth the extra assertion; `err != nil` would have
passed happily.
Third attempt at the same class of mistake, so worth naming precisely.
TestRoutesRegisteredInMount calls Mount() from INSIDE package api, so the
call reads `Mount(...)` unqualified. My verification grep was `api.Mount(`,
which cannot match it. Same shape as the previous failure, where I grepped
`auth.ClientIP(` and missed nothing — but only because those callers happened
to be in other packages.
The lesson generalises: after changing an exported signature, search for the
bare identifier, not the package-qualified form. In-package callers — which
in Go means most tests — are invisible to the qualified pattern.
This time I swept every signature I touched (Mount, RequireUser, ClientIP,
TouchSessionLastSeen) with an unqualified pattern before pushing, rather than
letting CI enumerate them one per run.
Passing h.netSettings (nil in test handlers) is deliberate, not a placeholder:
this test asserts route registration, and Hops() is nil-safe by design so the
middleware reads "trust nothing" rather than panicking.
Also gave netsettings' logger field a use — it was assigned and never read,
which staticcheck's unused pass can flag. A hop-count change alters how much
of a client-supplied header the server believes, so it earns a log line for
anyone later debugging odd addresses in the sessions list.
Two CI failures from 381e9ced, both mine.
**Go (vet, which cascaded into the integration job).** Widening
auth.ClientIP to take a hop count, I updated the middleware that TOUCHES a
session but missed the two places that CREATE one — handleLogin and
handleRegister. So `created_ip`, the frozen origin address that the whole
"address changed" comparison rests on, was the one value still being
computed the old way. Both now read h.netSettings.Hops(), which is nil-safe
so test handlers constructed without the service still work.
Worth noting the shape of this miss: I checked call sites by searching for
the middleware's own usage and stopped there, rather than for every caller of
the function whose signature I changed. vet found it in seconds; a grep for
`auth.ClientIP(` would have too.
**Web (vitest).** Three tests waited on `findByText('198.51.100.7')`, which
matches TWO elements in the fixture — the detected client address and the
forwarded chain, identical strings for a single-proxy setup — and findByText
throws on multiple matches. Now they wait on the unique "Your address right
now" label and assert the address with getAllByText where duplication is
legitimate. The duplication is correct behaviour, so the test moved rather
than the component.
Fixes the defect the operator spotted in #370 immediately after it shipped:
auth.ClientIP ignored X-Forwarded-For whenever RemoteAddr was public, so a
proxy on a public address — a separate host, or a CDN, i.e. anyone running
this publicly, since public means TLS means a proxy — recorded the PROXY for
every session. created_ip and last_ip were then always equal and the
"Address changed" signal could never fire. The feature looked like it worked
and reported nothing.
Replaced with the standard trusted-hop model (Rails, Caddy, Traefik, nginx).
XFF grows left-to-right as each proxy appends the peer it received from, so
for client -> CDN -> own-proxy -> app the app sees [client, CDN] with
RemoteAddr = own-proxy, and the client sits at XFF[len - hops]:
0 RemoteAddr, XFF ignored — no proxy
1 the address your own proxy observed
2 through a CDN in front of your proxy
Default 1, per the operator: publicly reachable means a TLS terminator in
front.
The cost is real and stated rather than hidden. hops >= 1 DECLARES that a
proxy exists; set it with no proxy, or deeper than the actual chain, and the
index reaches attacker-supplied entries, letting a visitor choose which
address their own session shows — defeating exactly the detection #370 is
for. That's inherent to the model, which is why 0 is a first-class value and
the admin card says "count your proxies, don't guess high" instead of just
exposing a number. Both mis-set shapes are pinned by tests so they stay known
consequences rather than surprises.
Migration 0053 + internal/netsettings, cached under an RWMutex. That's not an
optimisation: ClientIP runs in RequireUser for every authenticated request, so
a per-request query would put the database on the critical path of the whole
API. New() always returns a usable service so a boot-time DB hiccup degrades
to the default instead of breaking that path (rule #131), and Hops() is
nil-safe because test routers construct middleware without it.
RequireUser now takes a func() int rather than an int — the value is
operator-editable at runtime while the middleware is built once at boot, and
reading it per request is what makes a save take effect with no restart
(rule #25).
The admin card is verifiable, not just configurable: it reports the address
the CURRENT setting resolves THIS request to, the raw forwarded chain, and the
socket peer — so you set the number, save, and confirm the address matches the
machine you're on. It also counts the arriving chain and says how many proxies
that implies. GET/PUT both return that payload, PUT recomputed under the new
value, so the effect is visible without a reload.
Also fixes styling in the #370 card that CI could not catch: text-destructive
and bg-destructive don't exist in this Tailwind config — the palette is
colors.action.destructive — so the "Address changed" warning and the
sign-out-others button were rendering unstyled. Both now use
text-action-destructive / bg-action-destructive / text-action-fg.
Not done here: requestlog.go still logs raw RemoteAddr and will disagree with
the sessions UI about who connected. Left for its own change.
Client half of #370. Lists every device signed in to your account, with a
per-row sign-out and a "sign out all other devices" action.
The card does one thing the API alone doesn't: it says "Address changed" when
created_ip and last_ip differ, rather than printing two addresses and leaving
you to compare them. That mismatch — same device string, different origin —
is the shape of a stolen token, and it's the reason IP capture was worth a
migration. Making the operator spot it by eye would have wasted the data.
Placed with Password and API Token rather than at the bottom of the page:
those three are the account-security group, and this is the one that tells
you the other two need attention.
Details worth naming:
- The current session gets a "This device" badge and NO sign-out button —
offering one would log you out of the page you're standing on. The server
already excludes it from logout-others; this makes that visible.
- Sign-out-all-others is a two-step confirm and states the count, so the
button can't be a surprise.
- A 404 on revoke reloads instead of erroring. It means the session is
already gone — revoked elsewhere, or expired — so the list was simply
stale and showing the truth is the right response. The code is
`session_not_found`, not `not_found`: apierror.NotFound(what) prefixes it.
- Empty and error states both handled (rule #24); the empty case is
practically unreachable since listing requires an authenticated request,
and is handled rather than assumed.
- User-agent parsing is deliberately coarse. A real UA parser is a
dependency and a maintenance burden for a string whose only job is "do you
recognise this?" — the addresses carry the actual signal.
Tests cover the parts that would be quiet if broken: the current-session
badge suppressing its own sign-out button, the address-changed warning
appearing and NOT appearing, the two-step confirm not firing on first click,
and the load-failure retry.
Android parity is a separate decision, not assumed.
Server half of the active-sessions surface. Web UI follows.
The operator wants this specifically to notice a compromised account, which
sets the bar: the addresses have to be trustworthy, or the feature is worse
than absent because it looks like evidence.
Migration 0052 adds created_ip + last_ip. Two columns, not one, and the pair
is the signal: a session issued at home and now being used from elsewhere is
the shape of a stolen token, and neither column alone can show that. Typed
text, matching the user_agent column beside it — these are displayed, never
queried by subnet, and inet round-trips through pgx as a netip.Prefix that
renders "1.2.3.4/32".
The rest of the schema was already waiting. Migration 0004 anticipated this
exactly: "last_seen_at enables an 'active sessions' UI later (not wired in
this plan) without schema churn." last_seen_at is live data — the auth
middleware already touches it per request — so last_ip rides that same
UPDATE for free.
Getting the address right is the substance here. Nothing extracted a client
IP anywhere before, and both obvious approaches are wrong:
- RemoteAddr alone shows the reverse proxy on every session, which is the
normal self-hosted deployment. Noise shaped like data.
- Trusting X-Forwarded-For lets any client choose what its victim sees. A
security surface an attacker can write to is worse than none.
So auth.ClientIP trusts the header only when the request actually arrived
from a proxy range. Public RemoteAddr means a direct connection, so XFF is
attacker-controlled and ignored outright. Private RemoteAddr means we walk
XFF right-to-left — proxies append, so the right end is what our own
infrastructure wrote — and take the first non-proxy address. A forged XFF
only prepends to the left end, which that walk never reaches. Unit-tested,
including both spoofing shapes.
Fails closed on a public-addressed proxy (separate host, CDN): we report the
proxy rather than trusting a forgeable header. Documented at the function.
Endpoints, all scoped by user_id per rule #47:
GET /api/me/sessions → list, flagging the current row
DELETE /api/me/sessions/{id} → 204, or 404 if not yours
POST /api/me/sessions/logout-others → {"revoked": n}
Keyed on session id alone, any household member could revoke another's
session by guessing a uuid, so the delete carries user_id in its WHERE and
:execrows distinguishes "not yours" (404) from a false 204. There's a test
that asserts the row actually survives, not merely that we returned 404.
The middleware now also puts the session id in context. logout-others is
defined by exclusion, and without knowing which session is ours the
safe-looking action deletes everything including the caller's — so it
refuses rather than guesses when the id is absent, and that refusal is
tested for non-deletion too.
audit_log.action is plain text with no CHECK, so the two new actions need no
migration (rule #36 checked, not assumed).
Codegen is real sqlc 1.31.1 via the container in `make generate` — docker is
present on this workstation even though Go and sqlc aren't — rather than the
hand-written .sql.go shortcut used in milestone #268.
`android:usesCleartextTraffic="true"` sat on <application> as a bare opt-out of
the platform's network-security default, with nothing recorded about why. It's
now a res/xml/network_security_config.xml carrying the same permission and the
reasoning behind it.
Behaviour is unchanged. networkSecurityConfig supersedes the attribute on API
24+ and our minSdk is 26, so the attribute is removed rather than kept
alongside.
Cleartext stays permitted because two independent things need it, and neither
can be narrowed to a domain list:
- The Minstrel server's host is user-entered at runtime, and plenty of
self-hosters run plain HTTP on a LAN.
- UPnP/DLNA/Sonos — device-description and SOAP control URLs arrive in SSDP
responses at runtime and are plain HTTP essentially always. This one wasn't in
the original ticket, which only considered the server; it independently rules
out the "tighten it later to RFC1918" idea, since <domain-config> matches
literal hostnames, not CIDR ranges, and renderer IPs are unknowable ahead of
time.
Trust anchors deliberately left at the platform default. Adding
<certificates src="user" /> would let self-hosters use HTTPS with a private CA —
which Mihon does, and which suits this product — but it also trusts every CA on
the device including a corporate MITM proxy. Raised separately rather than
assumed as a default.
tools:ignore="InsecureBaseConfiguration" mirrors Mihon's config and keeps
lintVitalRelease quiet about a choice that is deliberate and now documented.
Minstrel integrates with Lidarr, and that integration is the kind of thing a
reader can misread as content sourcing. It isn't, and the README never said so
explicitly. Now it does, before the Quickstart rather than buried at the bottom.
The section states what is simply true: Minstrel indexes files already on disk
and streams them; it ships no indexers, no trackers, no torrent/Usenet/NZB
client, and no DRM circumvention; the Lidarr integration is optional, inert
until an operator supplies a URL and API key, and points at an instance they
already run. Library contents and configured sources are the operator's
responsibility. Plus a non-affiliation line for Lidarr, ListenBrainz,
MusicBrainz and Subsonic.
Also made the Lidarr highlight explicit that the instance is yours and the
integration is off by default — the bullet previously read as though Minstrel
brought Lidarr with it.
Written as scope-setting rather than legalese, deliberately: a confident
description of what the software does is both more useful to a reader and
better evidence of intent than an anxious disclaimer would be.
Docs only — no workflow path filter matches README.md, so no CI lane runs.
Replaces the ACTION_VIEW + application/vnd.android.package-archive handoff
with a PackageInstaller session, and declares
UPDATE_PACKAGES_WITHOUT_USER_ACTION so the update can land with no confirm
dialog at all.
The platform grants the silent path when the installer opts in via
setRequireUserAction(USER_ACTION_NOT_REQUIRED), the installed app targets
API 29+, the installer holds that permission, and the target is the
installer itself. Minstrel updating Minstrel satisfies all four. Where it
can't be granted — anything pre-S — the platform returns
STATUS_PENDING_USER_ACTION and we show its dialog instead, so this degrades
rather than failing.
Prior art: Mihon, which is out-of-store and self-updating and whose updates
are quiet for exactly this reason. It also confirmed REQUEST_INSTALL_PACKAGES
is not what draws install warnings — Mihon declares it too.
No setRequestUpdateOwnership(true), despite it reading like the obvious
declaration for a self-updater. Ownership can only be claimed on initial
installation (a no-op on update) and additionally wants the privileged
ENFORCE_UPDATE_OWNERSHIP permission. It's an API for app stores claiming the
apps they install.
Also: the install now has an outcome. The old path fired an intent and
assumed, so a failure and a user declining were indistinguishable. Sessions
report back, so InstallOutcome distinguishes Installed / Cancelled / Failed,
and cancelling returns to IDLE rather than showing an error — the user chose
it. DOWNLOADING and INSTALLING became separate stages because the install
half now genuinely waits, and "Downloading…" through a confirm dialog is a
lie.
The FileProvider and res/xml/file_paths.xml are gone. They existed only to
expose the cached APK as a content:// URI for the old intent; a session
takes a stream. Nothing else used that authority.
Two judgement calls worth naming:
- The pending-user-action intent is only launched if it resolves to a system
component. Below API 34 a dynamically registered receiver can't declare
itself unexported, so another app can broadcast at us, and an unchecked
startActivity on an attacker-supplied extra would be an escalation
primitive. The real confirm activity is a system app, so the check costs
the legitimate path nothing.
- Cancellation unregisters the receiver but deliberately does NOT abandon the
session. By then it's committed, and killing an install because the user
navigated away from the banner misreads their intent.
Untestable here: no androidTest source set and no Robolectric, so the
gesture-level behaviour is operator on-device verification.
detekt TooManyFunctions: the swipe work took the file to 12 functions
against a limit of 11. Suppressing it was the option; splitting is the
better one, because the seam was already there — the row carries two
gestures, a swipe background, and its own accessibility surface, which is
more behaviour than the screen that merely lists it.
QueueScreen.kt keeps the screen, list, pill, and summary (4). QueueRow.kt
takes the row and its helpers (8). No behaviour change: same code, same
order, per-file imports recomputed, QueueRow internal so QueueList can
still call it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N6vZoJ4Se5YyaqdtGVkap5
Replaces the trailing X button on the Android queue row, for the same
reason #2395 replaced the grip: horizontal space in the narrowest row in
the app. Web keeps its X — the operator's call, and the right one, since
the constraint being solved doesn't exist there.
SwipeToDismissBox with enableDismissFromStartToEnd = false; a right-swipe
means nothing here and would only delete tracks on a mis-aimed gesture.
The red fill under the row is oxblood (LocalActionColors.destructive), not
colorScheme.error — the design system keeps those apart because an error
is a failure that happened and a destructive action is one about to.
Adds a "Remove from queue" custom accessibility action. Both gestures the
row now relies on are touch-only, and each replaced a control TalkBack
could find, so without this the change would have quietly removed
remove-from-queue for anyone not using touch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N6vZoJ4Se5YyaqdtGVkap5
QueueRow hit 61 statements against detekt's 60 — the semantics block I added
for the screen-reader move actions pushed it one over.
Extracted to a `Modifier.queueReorderActions` extension, which mirrors the
`queueReorderDrag` extension from the same change: the row now composes two
named modifiers, one for the gesture and one for the accessibility actions,
instead of carrying either inline. Better than suppressing the rule — the
suppression would have been permanent and the split reads better anyway.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The grip icon took a column out of every queue row, competing with the title
for space — worst on Android, where the row is narrowest and the icon plus
its 12dp gap cost roughly 36dp. Operator pre-approved dropping the icon and
making the album art the drag surface; that's what this does.
## Android: the gesture change is the load-bearing part
Moved the drag from the grip onto the thumbnail AND switched
detectDragGestures → detectDragGesturesAfterLongPress. That second half is
not cosmetic. The grip was a small target, so a plain drag detector on it
never competed with anything; a 48dp thumbnail is a large chunk of every
row, and with a plain detector any vertical pan starting on artwork would be
swallowed as a reorder instead of scrolling the queue. The list would have
felt broken exactly where it's easiest to touch. Long-press-then-drag
separates the three gestures: pan scrolls, long-press reorders, tap still
plays (the detector doesn't consume a plain tap, so it reaches the row's
clickable).
Dropping the grip also removed its contentDescription ("Reorder track"),
which was the ONLY thing telling a screen reader this list could be
reordered — and a long-press drag isn't operable with TalkBack regardless.
Added "Move up"/"Move down" custom accessibility actions on the row, the
Android counterpart to the web row's ArrowUp/ArrowDown. Without them this
change would have quietly removed reordering for anyone not using touch.
## Web: the grip was never the drag surface
`use:draggable` is on the row, not the handle, so dragging already worked
from anywhere — the grip's only unique jobs were being the visual cue and
the keyboard target. It now sits OVER the art, costing zero horizontal
space, and keeps both jobs.
Deliberately still VISIBLE at rest, just quiet, with the scrim appearing
only on hover/focus. Overlaying already solved the space complaint, so
hiding it buys nothing and would cost the only cue that the queue is
reorderable — on touch especially, which has no hover.
## Scope walked back
Also considered the web PlaylistTrackRow, which carries an identical grip.
Left alone: it has no album art, so the approved direction doesn't apply,
and its handle is already the smallest of the three at 14px. Forcing
consistency would have meant inventing a third treatment for a surface
nobody complained about. (Android has no playlist reorder at all — that
parity gap is pre-existing and out of scope here.)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A Didone M whose right leg is an eighth note: stem, flag and notehead in the
accent, the letter in parchment. Traced from the operator's reference at
99.74% IoU (potrace, 26 + 22 segments), so the geometry is theirs, not an
approximation of it.
Subject-neutral on purpose. "Minstrel" pulls toward a lute or a bard, which
would tell a new user this is a renaissance-faire player rather than one for
all music. A geometric letter plus universal notation says "music" without
saying which music. The family look arrives through palette and drawing
style instead of through the subject — see the design-system discussion.
Starting state: web/static/favicon.png was a 1x1 PIXEL placeholder, so there
was effectively no favicon at all; Android had legacy bitmaps only, so modern
launchers letterboxed the square instead of masking it.
## The colour problem, and why each surface differs
Parchment on white is invisible — the operator caught this. The M therefore
has to flip with its background, while the accent note holds in both:
- mark.svg / MinstrelMark.svelte use currentColor, so the letter takes the
surrounding text colour and one asset covers both palettes.
- favicon.svg bakes colours with a prefers-color-scheme swap, because a
favicon sits on browser chrome and has no cascade to inherit from.
- PNG fallback, apple-touch-icon and Android are PLATED. A PNG can't
respond to scheme and iOS composites onto white regardless.
MinstrelMark is inlined rather than <img src>, because an <img> cannot
inherit currentColor and inheriting it is the entire point.
## Plate colour chosen by measurement
Obsidian (#14171A), not the raised-surface iron. The accent note only clears
the 3:1 non-text contrast threshold against the darker value: 3.04:1 vs iron's
2.70:1. My own earlier suggestion — lighten the plate — is WRONG and the
numbers say so: slate scores 2.21:1, worse, because the note is a dark colour
and lifting the plate closes the gap. Recorded in colors.xml so the reasoning
sits with the value.
## Construction
Traced as a full ink silhouette with the note painted OVER it, rather than as
two separate shapes. Separate shapes needed either a 2px seam where letter and
note touch, or an anti-aliasing fringe (2,430 misclassified pixels) around the
note. Painting over avoids both and yields a monochrome version for free — the
base layer alone is the whole mark in one colour, which is what
mipmap-anydpi-v26's <monochrome> uses for themed icons.
Android foreground sits at 61% of the 108dp canvas so it stays inside the
66dp safe zone and no launcher mask can clip it.
Paths are duplicated between the component and the two static SVGs, since one
needs currentColor and the others need literals. A comment in each names the
others.
Verified by render at 16/20/32/64/180 on obsidian, white, parchment and
plated; one optical size holds across the whole range.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"Matches your taste in shoegaze and dream pop." replaces the seed
attribution when the candidate's own tags overlap the taste profile.
The preference order is the point of slice 6: the tag reason describes the
MUSIC ("sounds like what you like"), while seed attribution describes the
graph ("adjacent to something you played"). When we can say the former, it
is strictly the better explanation. When we can't — the common case, since
tag coverage for out-of-library artists is partial by nature (#2376) — the
card falls back to attribution rather than going blank.
Both clients share the wording, Oxford comma included, and both have tests
asserting the exact strings. That's deliberate: identical copy across two
codebases silently diverges unless something fails when it does.
Android caps at 3 tags client-side even though the server already does.
The server contract could widen; a run-on subtitle shouldn't be how we
find out.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rule #25/#27: the two knobs slice 6 added server-side are now touchable —
taste-tag weight and snooze length, with deviation dots, save, and reset,
matching the existing profile/taste cards.
Copy states what each knob does AND what it doesn't: the tag-weight hint
says 0 turns the term off and that an untagged candidate is never
penalised, and the snooze hint says it records no opinion about the artist
and never feeds the taste profile. Those are the two properties most likely
to be assumed backwards by whoever turns these next.
Also fixed a latent fragility the new card exposed rather than caused: all
three reset buttons had the accessible name "Reset to defaults", so the
existing test picked the LAST one and assumed that meant taste. Adding a
card below it would have silently retargeted that assertion at the wrong
scope. Each reset button now names its scope — better for screen readers
too, since three identical buttons on one page is a real a11y defect — and
the test selects by name instead of position.
The page's test fixture needed the new `discover` key in both `snapshot`
and `shipped`: the `as TuningSnapshot` cast means a missing field is not a
compile error, it's every test on the page throwing inside fillForm. Noted
that in the fixture so the next scope doesn't rediscover it.
Includes a test that a weight of 0 is actually SENT rather than dropped as
falsy — the off switch is the one value a truthiness bug would eat.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TestSuggestArtists_UntaggedCandidateSurvivesAlongsideTagged asserted the
untagged candidate's score was 0.9 — the raw similarity value I'd seeded.
It's actually 1.61, because the pool score is signal-weighted by the seed
query: ln(1+signal) x similarity, and a liked seed carries signal 5, so
ln(6) x 0.9.
The assertion was testing the seeding arithmetic, which is a different
layer and not what the test is about. Rewritten to run the same request
twice — once with the tag term disabled, once enabled — and assert the
untagged candidate's score is IDENTICAL across both. That states the real
property (the blend leaves untagged candidates alone) without depending on
how the pool score is derived, so it survives future changes to seeding.
Added a sanity assertion that the TAGGED candidate's score did move, so
the comparison can't pass by both runs being trivially identical — the
same "a test that cannot fail" trap recorded for this milestone.
Exact-preservation at the arithmetic level is already covered where it
belongs, by TestApplyTagOverlap_UntaggedCandidateScoreIsUnchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The payoff slice. Until now a candidate's only claim on a slot was "some
artist you play is adjacent to it in a similarity graph" — a fact that says
nothing about whether the music sounds like anything you like. Now the
candidate's own folksonomy tags (cached by slice 5) are compared against
the user's taste-profile tags, so the deck ranks on taste and can say WHY.
The blend is MULTIPLICATIVE — score × (1 + weight × overlap) — and that
choice carries the whole safety argument:
- An untagged candidate has overlap 0, so its score is EXACTLY unchanged.
Tag coverage is permanently partial (#2376); it must cost a candidate
nothing, not sink it (rule #131).
- Nothing can leapfrog on tags alone. An additive term with a large
weight would let a near-zero-similarity artist outrank a strong match
for sharing one popular tag, which reads as noise.
- Weight 0 restores pure similarity order bit-for-bit, so the operator's
knob has a real off position.
overlap = Σ(shared) candWeight × normalizedTasteWeight ÷ Σ(all) candWeight.
Normalizing the taste side by the user's strongest tag makes the score
comparable across users (taste weights accumulate with listening, so a
heavy listener's raw numbers dwarf a new user's while meaning the same
thing). Dividing by the candidate's own mass makes it comparable across
candidates, so a densely-tagged artist can't win on tag count alone.
Applied to the whole over-fetched pool BEFORE selectSuggestions, so the
rotation and diversity rules operate on blended scores — boosting only the
twelve already chosen by similarity would leave the re-ranking undone.
A query failure is returned, NOT degraded past. Graceful degradation is
for expected absence (no taste profile, no cached tags) and both are
handled explicitly as empty inputs; swallowing a real error would hide a
broken DB behind a subtly worse ranking that nothing reports.
Migration 0051 adds a FOURTH tuning scope rather than columns on
taste_tuning, because snooze_days lives here too and a snooze must never
be read as taste signal (#2374) — filing it under 'taste' would put it one
careless join from the leak that design forbids. Expanding
recommendation_tuning_audit's CHECK is in the same migration per rule #36,
and a test asserts the audit row lands, which is what would catch its
absence.
snooze_days moves out of a Go constant onto the tuning card (rule #25),
closing the deferral from #2374.
Tag-overlap tests use deliberately SKEWED fixtures: an evenly-matching pool
cannot exercise a re-ranking, since every candidate gets the same
multiplier and the order is unchanged whether the blend works or not.
Admin UI + client attribution follow in this batch — rule #27.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three divergences in the hand-written generated file, all caught by
verify-generate on the first run. Two are sqlc rules I had wrong:
1. When a query's SELECT list exactly matches a table's columns in order,
sqlc REUSES the model struct rather than emitting a bespoke Row type.
So ListCandidateArtistTagsForMbids returns []CandidateArtistTag, and
ListCandidateArtistTagsForMbidsRow should never have existed.
2. models.go is ordered by GO STRUCT NAME, not table name. Table order
would put candidate_artist_tag_state before candidate_artist_tags;
sqlc emits CandidateArtistTag before CandidateArtistTagState. The
earlier slice-3 observation ("ordered by table name") was consistent
with both orderings and so never discriminated — this case does.
3. sqlc smart-quotes a doubled '' inside a promoted comment into a
typographic ”. Reworded the prose to say "the empty string" instead of
encoding a mangling into the source.
Note the integration lane PASSED on the broken push while this failed.
That is #2380's lesson landing again, and the reason the check exists:
valid SQL executing against real Postgres proves nothing about whether
the committed Go matches its source.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Migration 0050 adds candidate_artist_tags + candidate_artist_tag_state:
folksonomy tags for artists NOT in the library, which track_tags cannot
hold because it's FK'd to tracks(id) and a Discover candidate has no local
row. Slice 6 ranks against these; this slice only fills the cache.
The reuse the task claimed is real and verified: MusicBrainz's
fetchEntityTags(ctx, "artist", mbid, scale) already existed for the #1519
recording→artist fallback, so FetchArtistTags is a thin wrapper. Two
subtleties it does NOT inherit:
- Weight scale is 1.0, not artistTagWeightFactor (0.6). That discount
exists because FetchTrackTags uses artist tags as a *proxy* for a
track's; here the artist IS the subject. Applying it would make these
weights incomparable with track_tags — exactly the comparison slice 6
depends on. Pinned by a test.
- fetchEntityTags reports existing-but-untagged as (empty, nil) so the
track path can fall through. There's no next level here, so empty
becomes the terminal ErrNotFound; otherwise the enricher would settle
a candidate as "enriched" with zero tags.
ArtistTagProvider is the split TrackTagProvider's own doc comment
anticipated ("e.g. artist-level tags"). Last.fm gains artist.getTopTags,
which returns the same toptags envelope, so the response type and
normalizer are reused unchanged.
Rather than write the merge-and-classify loop twice, extracted it from
EnrichTrack into runChain(). The ErrNotFound-vs-transient split is the
load-bearing part — those lead to opposite persistence decisions — so it
now has direct unit tests it never had while inlined.
Bookkeeping is a separate table, not columns, because the "providers had
nothing" outcome must be recordable for a candidate with zero tag rows,
and there is no per-candidate row to hang columns off (
artist_similarity_unmatched holds many rows per candidate). Absence of a
state row means "never processed", so a transient failure writes nothing
and stays eligible.
Two capacity realities are designed for, not papered over:
- The pool is O(library artists x neighbours) and MusicBrainz allows
~1 req/s, so it can never drain in one pass. The eligibility query
returns candidates in descending summed-similarity order, so the ones
that can actually reach a deck are enriched first.
- candidateBatch (50) is smaller than the track batch (200): tracks are
finite and drain to completion, candidates are effectively unbounded
and would otherwise starve the track arm forever.
GC sweeps both tables — the similarity feed churns, and a candidate that
joins the library has its tags in track_tags now. Tags swept before state
so a mid-sweep crash leaves a valid state, not a re-fetch loop.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The days→months threshold (45) sat above the divisor (30), so a rounded
month count of 1 — which needs 15..44 days — could never be reached: every
one of those day counts hit the `in N days` branch first. The singular
branch was dead code on Android AND web.
Lowered the threshold to 30 in both clients, which makes 30..44 days read
"in about a month" instead of "in 44 days", and documented the invariant
(threshold must not exceed the divisor) next to each constant so the two
can't drift apart again.
Found by the unit test written for that branch, which is the whole reason
to assert on copy that looks obviously correct. Both suites now pin the
seam from both sides — 29 days and 30 days — so the branch can't go dead
again silently.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two CI failures from 6e39471a, both mechanical.
web: src/routes/discover/discover.test.ts mocks $lib/api/suggestions with
a factory, and SuggestionFeed now imports createSnoozesQuery from it. A
factory-shaped module mock must export everything the component tree
imports or rendering throws before any assertion runs — so all 12 of that
suite's tests failed on a surface they don't even exercise. Stubbed the
three new exports and defaulted the snooze query to empty, which keeps
the feed's empty-state copy on the "no signal yet" branch those tests
assert. (Same shape as Scribe #2109: when a shared component grows a
dependency, the break is in unrelated fixtures, not assertions.)
android: detekt ReturnCount — returnsIn had 3 returns against a limit of
2. Folded the two "nothing to state" guards into one by computing the
remaining duration as a nullable up front.
The Android compile and unit tests never ran on the last push: detekt
gates them, so Lucide.Clock is still unproven.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Completes the snooze from slice 3 (#2374), so it's now touchable on both
clients (rule #27 — the server side alone was never shippable).
Copy is "Not right now" everywhere, never a dislike (rule #101). The
parked list even says so out loud: "Nothing here counts against your
taste profile."
Both clients flip the card in place to a "Not right now" state with an
Undo, rather than yanking it out of the grid under the cursor. The row
leaves on the next refetch; the persistent way back is a parked-list
section below the deck. That list isn't optional garnish — a snoozed
candidate is by definition absent from the deck, so without it the
DELETE endpoint is unreachable.
Android routes the write through the offline MutationQueue per rule #100,
as ONE toggle kind (SUGGESTION_SNOOZE_TOGGLE) carrying the desired state
rather than two action kinds. That reuses the LIKE_TOGGLE collapse: a
queued snooze the user has since undone is dropped unsent instead of
replaying after the undo and re-hiding an artist they asked to see. The
collapse helper is now a pure top-level function so that rule is unit
tested rather than inferred.
The repository does NOT enqueue on a 4xx — a permanent rejection would
replay to the same failure and would raise a misleading "will sync when
online" hint. The common case is a 404 from un-snoozing a row that
already lapsed, which is the user's intended end state anyway.
Also: an empty deck used to have one meaning (no listening signal yet).
It can now also mean "you parked them all", so the empty copy branches —
telling that user to go listen to something would be wrong advice.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Migration 0049 adds suggestion_snoozes(user_id, candidate_mbid,
candidate_name, snoozed_until), and SuggestArtistsForUser excludes rows
whose snooze hasn't expired.
This is NOT a dislike. Rule #101 forbids a "Not for me" / thumbs-down
UI; a snooze is the approved shape instead because it records no verdict
on the music, expires on its own (~90d), and never reaches the taste
profile. It's acquisition triage — "not right now" — so the filter sits
at the candidate stage rather than in the score, where it would become a
ranking signal by the back door.
Per-user throughout (rule #47): one household member parking a candidate
leaves everyone else's deck untouched.
candidate_name is denormalized because suggestions are out-of-library by
definition — there is no artists row to resolve a display name from, and
the un-snooze list has to show something. That list is why GET
/discover/snoozes exists at all: a parked candidate is by definition
absent from the deck, so without it the DELETE would be unreachable.
Also fixes a hole in the codegen check from #2380: `git diff` ignores
untracked paths, so a brand-new generated file would have passed it
silently. `git add -N` first. This commit is the first to add one.
Endpoints:
POST /api/discover/suggestions/{mbid}/snooze (body: name, days)
DELETE /api/discover/suggestions/{mbid}/snooze
GET /api/discover/snoozes
UI lands in slice 4 (#2375) before any of this merges — rule #27.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The new codegen check failed on its first run, against the slice-1 hand-edit,
which is precisely why it landed on its own commit.
What I got wrong: sqlc does not embed the leading `--` header block in the SQL
const. It strips those lines and promotes them to the generated method's Go doc
comment, gofmt-formatted — blank `//` separators around the indented list, tabs
for the indent. My hand-edit left the header inside the string AND left the
stale M5c doc comment sitting on the function, so the generated file described
behaviour the query no longer had.
Comments *inside* the statement body are kept as-is; only the header block moves.
Worth knowing before slices 5 and 6 add more queries.
Taken verbatim from the diff the check printed, which is the reason it prints
before asserting. Round-trip cost: one CI run, no guessing.
Note the integration lane passed on the previous push even with the wrong
generated file — the SQL text was valid and the signature was unchanged, so
executing it against real Postgres proved nothing about whether the committed
Go matched its source. That gap is exactly what #2380 closes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
internal/db/dbq is 39 files and ~12k lines of generated Go covering 307
queries, and nothing checked that it still matched internal/db/queries.
test-go.yml referenced sqlc.yaml only as a path trigger; sqlc never ran. So a
hand-edit, a half-applied regen, or a migration changed without a regen would
all pass CI while the typed layer quietly lied about the SQL underneath it —
which is the single thing adopting sqlc is supposed to buy.
This session's slice-1 change is an instance: its SQL const was verified
byte-identical against its own .sql source by script, but never against what
sqlc would actually emit. Nothing in the repo could have told the difference.
make verify-generate runs ahead of vet/lint/test, because if the typed layer
disagrees with its sources then everything downstream is testing a lie.
generate-go runs sqlc as a Go tool rather than a container: the ci-go image
already has Go, so this avoids docker-in-docker on the runner. It's pinned to
the same SQLC_VERSION as the existing containerised `generate`, so both routes
emit identical output and there is one version to bump — now annotated for
Renovate per rule #44.
The diff prints BEFORE the exit-code check on purpose. On failure the log then
holds sqlc's exact expected output, so correcting it is a copy rather than a
guess. That is also what makes new queries workable without installing
anything: this workstation has neither Go nor sqlc.
Makefile joins the workflow's paths:. Without it a Makefile-only change —
including this one — would not trigger the workflow that now depends on it.
Same class as #2204, where CI never ran on plugin/** changes.
Landing this on its own, ahead of slice 3, so that if it fails it is
unambiguous whether the drift came from slice 1 or from new code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Second half of the reported symptom: suggestions "show the same artists until
you request one". The ranking was `ORDER BY total_score DESC` with no
randomization and no seen-state, so the only things that could ever change the
deck were a candidate entering the library or the user filing a request. The
tail of the ranking was unreachable — requesting was literally the only lever.
No SQL change was needed. The query already takes a limit, so it over-fetches a
pool (4x the slots, capped at 60) and the selection moves to Go, where it is a
pure function of (pool, limit, day) — no DB, no clock — and therefore unit
testable in the fast lane instead of behind the integration gate.
Three rules. The best few by score always lead, so the strongest matches never
rotate out of sight (For You's head/tail shape). The remaining slots are drawn
by md5(mbid + day), the same daily-stable idiom the Home rows already use:
stable within a day so pull-to-refresh doesn't reshuffle, different tomorrow,
and no stored state. And a per-seed cap keeps roughly a quarter of the deck
attributable to any one seed artist, so twelve neighbours of a single artist
can't be the whole surface.
The cap is a preference, not a quota. A user whose pool hangs off one or two
seeds would otherwise get a three-card surface — worse than the monoculture
being avoided, and exactly the vanish-or-nothing shape rule #131 exists to
prevent — so a short deck tops up in score order from what the cap set aside.
This is also what keeps the existing Top12Cap integration test honest: its
30 candidates share one seed, and without the top-up it would return 3.
Eight unit tests, including one that had to be rewritten mid-change: the first
version asserted the cap against an evenly-spread pool, where the top-N is
already diverse and the assertion could not fail. It now uses a skewed pool
where one seed owns the entire top of the ranking, which is the only shape that
actually exercises a cap.
Dropped two //nolint:gosec directives added in passing — gosec isn't in
.golangci.yml, so they suppressed nothing and only implied a check that runs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Discover request surface was the one recommendation surface still on its
M5c implementation from early May. #796's taste profile, #1488's taste_unheard
bucket and #1490's folksonomy enrichment all modernized in-library surfaces;
this one was never in scope for any of them, so it still projected raw likes +
plays through artist_similarity_unmatched.
Two defects fall out of that signal, `5*liked + Σexp(-age/halflife)` summed
over every play of the artist.
It is unbounded, and contribution is signal × similarity — so a handful of
heavily-played artists monopolize all twelve slots, and their share GROWS the
more the user listens. The surface entrenched harder the better it knew you,
which is exactly backwards and matches the reported "goes stale once it has a
strong signal of your taste".
It also counted every play_event with no was_skipped filter, so skipping an
artist repeatedly INCREASED its signal and pushed more of its neighbours at the
user. ListMostPlayedTracksForUser and the taste engine both filter skips; this
query was the odd one out.
Seeds now come from taste_profile_artists.weight, which the taste engine has
already engagement-graded, time-decayed and signed — an artist the user drifted
away from stops contributing instead of accumulating forever, and can even
contribute negatively. Tiered per rule #131 rather than hard-switched: tier 1 is
the profile, tier 2 is likes + completed plays for a user who has no profile
rows yet (new account, or before the first daily recompute), so the surface
never empties. The old unfiltered-play signal is gone, not kept behind a toggle.
The signal is also log-damped, so one artist cannot take every slot even when
its weight dwarfs the rest.
$2 stays wired to the tier-2 decay: it is genuinely still used there, and
dropping the parameter would have changed the generated signature.
sqlc's image is not on this workstation and the change preserves the query
signature exactly — same three params, same seven columns — so only the
embedded SQL const moves. Both copies are edited and verified byte-identical
rather than pulling a container onto the operator's machine; a malformed query
fails the integration lane loudly, which is the real check either way.
Four integration tests cover what changed: a taste weight alone seeds with no
like or play; tier 2 does not run alongside tier 1; a non-positive weight never
seeds (guarded by a second positive row, so an empty tier 1 can't make it pass
for the wrong reason); and skip-only history seeds nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`refreshDetail` did an un-transacted `deleteByPlaylist` + `upsertAll` — the
same shape as the Home index write that #2327 just fixed. Room's
InvalidationTracker fires after the DELETE, so an observer of
`observeByPlaylist` would see `emptyList()` before the new rows land, which is
exactly what made every Home row visibly collapse to empty and refill.
Nothing consumes `observeByPlaylist` today, so this is not a live defect — it's
a landmine. Making playlist detail cache-first later would have silently
reintroduced the flicker, and the reason would have been three layers away from
the symptom. One `@Transaction` now costs nothing and removes that.
`deleteByPlaylist` is left in place as the building block but is no longer
called from outside the DAO.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The sheet still described the pre-M8 world: "two CI images: ci-go +
ci-flutter", a ci-flutter dep list, and cross-workflow release polling
against flutter.yml. None of that is true now — flutter.yml is gone,
android.yml and release.yml both pull ci-android:36, and image-release
gates on `needs: [android-release]` instead of polling.
Family rule 39 makes this sheet CI-Runner's decision input for "add a dep
to an image vs. fork a variant", so a stale sheet quietly misinforms that
call: CI-Runner was still carrying ci-flutter for a consumer that no
longer exists, and had no record of ci-android's real consumer.
- Runtime images: ci-flutter:3.44 -> ci-android:36, with a note on why
ci-flutter is now unconsumed and what would have to change to revive it.
- Image deps: replace the Flutter/Dart/NDK list with the actual
ci-android surface (JDK 25 + Gradle 9.1 floor, SDK/build-tools 36, no
NDK, ktlint + detekt).
- Label/image split: record that Android jobs still schedule on the
flutter-ci label on purpose — it's a scheduling handle, not a toolchain
assertion.
- Update channel: `needs:` gating, plus the non-tag rebundle path.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 10:38:50 -04:00
591 changed files with 20600 additions and 25777 deletions
# Checked BEFORE the expensive work, not after it. "Attach APK to gitea
# Release" below resolves the release by tag and fails if it is absent —
# but that is the final step, so a tag pushed without a release built an
# APK for several minutes first and only then discovered it had nowhere to
# put it. Same check, seconds in instead of minutes.
#
# Releases are normally created through the API (which creates the tag and
# the release together, so this passes). A bare `git push origin vX` is the
# case this catches.
- name:Release must exist for this tag
shell:bash
working-directory:${{ github.workspace }}
env:
CI_TOKEN:${{ secrets.CI_TOKEN }}
run:|
set -euo pipefail
TAG="${GITHUB_REF#refs/tags/}"
if ! curl -fsSL -o /dev/null \
-H "Authorization: token ${CI_TOKEN}" \
"https://git.fabledsword.com/api/v1/repos/${GITHUB_REPOSITORY}/releases/tags/${TAG}"; then
echo "::error::no release exists for ${TAG}. Create the release (which creates the tag) rather than pushing a bare tag — otherwise there is nothing to attach the APK to."
echo "::error::no release found for ${TAG} — the tag exists but nothing was published"
exit 1
fi
APK="$(printf '%s' "${REL_JSON}" \
| grep -oP '"browser_download_url":\s*"\K[^"]+' \
| grep -E '\.apk$' | head -1 || true)"
if [ -z "${APK}" ]; then
echo "::error::release ${TAG} has NO APK attached — in-app update will offer nothing, and the bundled-APK path on future :latest builds has no source."
echo "::error::Fix by RE-RUNNING this workflow run. Do NOT delete and re-create the tag; if it fails again the runner never started the container, and the evidence is in act_runner on the host (Gitea will hold no job log)."
exit 1
fi
echo "::notice::APK attached: ${APK}"
# The other half. Checking only the APK would report success on a release
# whose image push failed — which is precisely the second thing that was
# missing when v2026.08.07 had to be re-cut. `always()` on this job means
# it runs even when image-release failed, so without this the guard would
if ! docker manifest inspect "${IMAGE}:${TAG}" > /dev/null 2>&1; then
echo "::error::image ${IMAGE}:${TAG} was never pushed — the release tag has no immutable image, so there is nothing to pin or roll back to. Re-run this workflow run."
@@ -11,10 +11,22 @@ A self-hosted music server that thinks for you. Smart shuffle, contextual likes,
- **OpenSubsonic-compatible.** Existing Subsonic clients (DSub, Symfonium, play:Sub, etc.) connect with no special configuration.
- **Server-side smart shuffle.** Track-similarity vectors, dual-like model (general + contextual), and session memory keep mixes coherent across devices.
- **ListenBrainz radio.** Session-aware "more like this" pulls from ListenBrainz similarity data, not a static genre tag.
- **Lidarr integration.** Triggered scans, request-driven album imports, and a quarantine flow when something doesn't fit.
- **Lidarr integration.** Triggered scans, request-driven album imports, and a quarantine flow when something doesn't fit — against a Lidarr instance *you* run and configure. Optional, and off until you supply a URL and API key.
- **Built-in web SPA.** Full-feature library, search, queue, playlists, and admin — no separate frontend container to deploy.
- **Native Android client, shipped with the server.** The signed APK is bundled into every image and attached to each [release](https://git.fabledsword.com/bvandeusen/minstrel/releases) — sideload it once, then the app self-updates straight from your own server (no app store, no separate download to track).
## Scope and responsible use
**Minstrel serves music you already have.** It is a library server: it indexes files on disk you point it at, and streams them to your own clients. It does not source, search for, or acquire content, and it has no opinion about where your files came from.
Concretely, Minstrel ships **no** indexers, **no** trackers, **no** torrent / Usenet / NZB client, and **no** DRM circumvention of any kind. There is nothing to point at a content source because Minstrel has no such subsystem.
The **Lidarr integration is optional and inert until you configure it.** You supply the URL and API key of a Lidarr instance you are already running; Minstrel then calls that instance's API to trigger scans, submit album requests, and reconcile imports. Minstrel neither bundles nor installs Lidarr, and configures no indexers on your behalf — Lidarr ships with none either, and any it uses are ones you added yourself.
**What you put in your library, and what sources you configure in your own Lidarr, are your responsibility.** Copyright law applies to your collection the same way it applies to any other software that plays a file. Please respect it, and respect the terms of any service you connect.
Minstrel is not affiliated with or endorsed by Lidarr, ListenBrainz, MusicBrainz, or Subsonic.
@@ -136,6 +173,12 @@ class PlayerFactory @Inject constructor(
.build(),
)
privatecompanionobject{
// Enough readings to hold a whole flap episode plus the normal
// transitions around it; the detector's window is only a few seconds.
constvalTRANSPORT_EVENT_BUFFER=32
}
privatefunemitDrop(routeName:String){
dropEventsInternal.tryEmit(routeName)
}
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
bvandeusen
