Missing-file lifecycle end to end, UPnP stall recovery, Android browse parity, Flutter client removed #126

Merged

bvandeusen merged 23 commits from dev into main 2026-08-17 16:28:14 -04:00

Conversation 0 Commits 23 Files Changed 414

+5415 -24882

bvandeusen commented 2026-08-17 16:25:41 -04:00

Owner

Copy Link

Copy Source

23 commits. Every one CI-green on dev; head is 955a6119 (run 3879, both lanes).

The missing-file lifecycle, completed — #2527, milestone #290

The through-line of this batch. missing_since existed but was a dead end: reconcile marked it, selection paths skipped it, and nothing else in the product knew. It is now a full lifecycle.

The design was corrected mid-flight. #2527 was written around deleting orphaned tracks. The operator's model is the opposite — nothing is ever deleted, the mark IS the state, and the file is expected to come back:

the intent of 'deleting' a track is to remove it from being played and to put the system in a state where something like lidarr will attempt to replace the file. we should be keeping records of the track the file was supposed to represent and all the history of how it was used.

So the row, its play history, its likes and its taste contribution all survive. That also dissolved the CASCADE risk that had made the original ticket cautious.

What shipped:

• Playlists keep the entry, greyed, unplayable, skipped — and the wire withholds stream_url, so the server refuses to offer a URL that cannot serve rather than trusting clients to honour a flag. Web and Android.
• Admin surface at /admin/missing-files, grouped by directory: the case behind #2523 was three reorganised albums, which a flat list renders as forty unrelated problems. Each row carries last_played_at, because "gone six months, never played" and "gone yesterday, played 200 times" deserve opposite reactions.
• Automatic re-acquisition (milestone #290): an hourly sweeper turns missing albums into Lidarr requests on an exponential backoff. Two decisions carry the safety — requesting the album rather than the track (three requests, not forty), and a grace period before the first attempt, because an unmounted volume resolves itself well inside a day. Settings are DB-backed with an admin card that states the resulting schedule, since "first retry gap: 6" means nothing until you know it doubles.
• Clients are told — #2704. The wire field alone was inert: MarkTracksMissing is a plain UPDATE and /api/library/sync is a change-log feed, so nothing ever asked a client to re-read the track. Reconcile now emits change rows, logged before the mutation because the failure modes are asymmetric (a spurious re-read costs one fetch; a missed change row means the client never learns, permanently).

UPnP playback stops in silence — #2700

Diagnostics showed a Sonos stopping by itself mid-track while the phone was in Doze, and the app reporting it faithfully for minutes without acting. RemoteStallWatchdog notices and retries with backoff, then reports a stalled playback error — a kind that has been in the schema since 0032 and had never once been emitted.

Deliberately conservative: STOPPED-or-error only (a PAUSE is a person at the Sonos app, and taking the transport off them is a fight they always lose), three polls must agree, three attempts per track then it gives up.

The underlying cause is still open — this makes a dropout survivable, it does not explain it.

Android browse parity — #2467

Genres and Years land in the Library, matching the web tab order. Server-backed rather than cache-first, which is the one real decision: the local cache could not compute a correct index until #2704, so it would have disagreed with the web client. Now that it can, those two tabs can go back to Room whenever the Android tabs are next touched.

The Flutter client is gone — #2710

245 files, ~24.6k lines, last touched 2026-05-31 and built by nothing since flutter.yml was removed. shared/fabledsword.tokens.json was kept and moved to the repo root — it lived under the client but was never its property. runs-on: flutter-ci deliberately stays: it is a runner label, not a path.

ci-flutter can now be retired in CI-Runner — ci-requirements.md said it would go once this directory did, and it has.

Also

• #2681 — the dead strip above the MiniPlayer: an unclaimed navigation-bar inset leaking into all 15 in-shell screens, plus 140dp of inherited Flutter-era padding.
• #2702 — a fully-missing album leaves the year axis, matching what genre already did. The two browse axes had been disagreeing.
• One DRY consolidation: relativeTime had three byte-identical copies; timeUntil joined it for countdowns. They disagree about the future on purpose, and a test says so.

What CI cannot sign off

• Lidarr actually re-downloading. There is no Lidarr in CI; the fast check is the admin Requests queue after a scan marks something past the grace window. auto_approve defaults ON.
• The stall watchdog against real hardware.
• The layout changes, which are device-eyeball.

23 commits. Every one CI-green on `dev`; head is `955a6119` (run 3879, both lanes). ## The missing-file lifecycle, completed — #2527, milestone #290 The through-line of this batch. `missing_since` existed but was a dead end: reconcile marked it, selection paths skipped it, and nothing else in the product knew. It is now a full lifecycle. **The design was corrected mid-flight.** #2527 was written around *deleting* orphaned tracks. The operator's model is the opposite — nothing is ever deleted, the mark IS the state, and the file is expected to come back: > the intent of 'deleting' a track is to remove it from being played and to put the system in a state where something like lidarr will attempt to replace the file. we should be keeping records of the track the file was supposed to represent and all the history of how it was used. So the row, its play history, its likes and its taste contribution all survive. That also dissolved the CASCADE risk that had made the original ticket cautious. What shipped: - **Playlists** keep the entry, greyed, unplayable, skipped — and the wire withholds `stream_url`, so the server refuses to offer a URL that cannot serve rather than trusting clients to honour a flag. Web and Android. - **Admin surface** at `/admin/missing-files`, grouped by directory: the case behind #2523 was three reorganised albums, which a flat list renders as forty unrelated problems. Each row carries `last_played_at`, because "gone six months, never played" and "gone yesterday, played 200 times" deserve opposite reactions. - **Automatic re-acquisition** (milestone #290): an hourly sweeper turns missing albums into Lidarr requests on an exponential backoff. Two decisions carry the safety — requesting the **album** rather than the track (three requests, not forty), and a **grace period** before the first attempt, because an unmounted volume resolves itself well inside a day. Settings are DB-backed with an admin card that states the resulting schedule, since "first retry gap: 6" means nothing until you know it doubles. - **Clients are told** — #2704. The wire field alone was inert: `MarkTracksMissing` is a plain UPDATE and `/api/library/sync` is a change-log feed, so nothing ever asked a client to re-read the track. Reconcile now emits change rows, logged *before* the mutation because the failure modes are asymmetric (a spurious re-read costs one fetch; a missed change row means the client never learns, permanently). ## UPnP playback stops in silence — #2700 Diagnostics showed a Sonos stopping by itself mid-track while the phone was in Doze, and the app reporting it faithfully for minutes without acting. `RemoteStallWatchdog` notices and retries with backoff, then reports a `stalled` playback error — a kind that has been in the schema since 0032 and had never once been emitted. Deliberately conservative: STOPPED-or-error only (a PAUSE is a person at the Sonos app, and taking the transport off them is a fight they always lose), three polls must agree, three attempts per track then it gives up. **The underlying cause is still open** — this makes a dropout survivable, it does not explain it. ## Android browse parity — #2467 Genres and Years land in the Library, matching the web tab order. Server-backed rather than cache-first, which is the one real decision: the local cache could not compute a correct index until #2704, so it would have disagreed with the web client. Now that it can, those two tabs can go back to Room whenever the Android tabs are next touched. ## The Flutter client is gone — #2710 245 files, ~24.6k lines, last touched 2026-05-31 and built by nothing since `flutter.yml` was removed. `shared/fabledsword.tokens.json` was kept and moved to the repo root — it lived under the client but was never its property. `runs-on: flutter-ci` deliberately stays: it is a runner label, not a path. **`ci-flutter` can now be retired in CI-Runner** — `ci-requirements.md` said it would go once this directory did, and it has. ## Also - `#2681` — the dead strip above the MiniPlayer: an unclaimed navigation-bar inset leaking into all 15 in-shell screens, plus 140dp of inherited Flutter-era padding. - `#2702` — a fully-missing album leaves the year axis, matching what genre already did. The two browse axes had been disagreeing. - One DRY consolidation: `relativeTime` had three byte-identical copies; `timeUntil` joined it for countdowns. They disagree about the future on purpose, and a test says so. ## What CI cannot sign off - Lidarr actually re-downloading. There is no Lidarr in CI; the fast check is the admin Requests queue after a scan marks something past the grace window. **`auto_approve` defaults ON.** - The stall watchdog against real hardware. - The layout changes, which are device-eyeball.

bvandeusen added 23 commits 2026-08-17 16:25:41 -04:00

fix(android): let list content reach the MiniPlayer — #2681 ... 20bd7bfaf8

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.

feat(library): a missing file stays in the playlist, greyed and unplayable — #2527 ... c3f3a17c6d

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.

feat(api): admin surface for files the library has lost — #2527 ... 4dd0a58d63

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.

feat(web): a playlist entry whose file is missing greys out and is skipped — #2527 ... 4c49ee2cc6

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.

feat(android): name the missing file behind a greyed playlist row — #2527 ... aab90a7a39

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).

refactor(web): one relativeTime for the triage surfaces — #2527 ... 845f45fb0b

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.

feat(web): admin page for files the library has lost — #2527 ... 8d1f2674fd

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.

test(web): admin nav is nine tabs — #2527 ... a31b672b14

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.

fix(android): notice when a Sonos stops on its own, and get it going again — #2700 ... d9238ec5be

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.

feat(android): Genres and Years browse axes in the Library — #2467 ... 3eada70aac

#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.

style(android): satisfy detekt on the new browse tabs — #2467 ... bfb6c9acfe

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.

chore: delete the Flutter client — #2710 ... 0036f534db

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.

docs: stop pointing at the deleted Flutter tree — #2710 ... 03a8d12079

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.

feat(library): a missing file asks Lidarr for itself, on a backoff — #2527 ... bab9b16831

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.

feat(api): admin endpoints for the re-acquisition policy — #2527 ... 30a5ac56ce

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.

test(api): cover the missing-file admin routes in the Mount test — #2527 ... c2862e97bd

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.

feat(web): re-acquisition settings card on the missing-files page — #2527 ... 952132714e

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.

feat: show what re-acquisition has done, per folder — #2527 ... 414dfb23b6

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).

fix(web): timeUntil rounds, so a 4h wait doesn't read as 3h — #2527 ... 6d729d1512

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.

fix: stop the sync feed hiding missing files from clients — #2704 ... 366692a1fc

#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.

fix(library): tell clients when a file goes missing or comes back — #2704 ... 7ba673ed83

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.

test(android): TrackRef needs albumId and artistId — #2704 ... b96285d6d9

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.

fix(library): a fully-missing album leaves the year axis too — #2702 ... 955a61194e

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.

bvandeusen merged commit 727f68950e into main 2026-08-17 16:28:14 -04:00

bvandeusen referenced this pull request 2026-08-18 10:50:04 -04:00

UPnP: verify the Sonos queue landed, stop the phone double-downloading, and instrument the stutter #127

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

deps

Dependency update (Renovate)

deps:major

Major dependency bump — held for manual review

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate-bot bvandeusen

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: bvandeusen/minstrel#126