a687ef439c5252919f967fda80db9d3dd513ac42
1889
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  bvandeusenandClaude Opus 5
|
a687ef439c |
fix(release): refuse an ordering key that overflows versionCode
The script asserted the key was positive but never that it fits. Android's
versionCode is a signed 32-bit int and the platform rejects an APK above it,
so a build machine with a badly wrong clock would emit a code the script
happily hands on and the install then refuses.
Worse than a rejected build: an over-ceiling code is also unreachably high,
so every correct build afterwards would fail to outrank it and the update
channel would be permanently stuck. Cheaper to refuse at the source than to
diagnose it from a phone that will not update.
The Go guard already asserted this, but only against a pinned value. The
script is what actually runs at build time, so the check belongs here too.
Falsified at the boundary rather than by eye — exactly at the ceiling exits
0, one minute past exits 1. My first probe used a year-6000 clock and did
NOT fire, which turned out to be the probe being wrong rather than the
check: that epoch still lands under the ceiling. The ceiling is reached in
6103, roughly 4079 years out, so this only ever catches a misconfigured
clock.
This commit deliberately touches ci/version.sh alone, to verify the path
filters added in
|
||
|
bvandeusenandClaude Opus 5
|
eaf4654c0a |
test(release): make the version derivation executable, and guard it on dev
Steps 1 and 2 of this milestone shipped with no CI coverage at all, and the
reason generalises: release.yml triggers only on main and tags, so nothing
inside it is exercised until a release is already running. That is the worst
place in the repo to be unguarded, because the failure mode is silence — a
version nobody can compare looks exactly like being up to date, and nobody
reports an update they were never offered.
The fix is not a test that reads YAML. The derivation moved into
ci/version.sh, so it can be RUN, and internal/server/release_version_test.go
runs it on every push. release.yml now calls the same script, so the thing
that ships and the thing under test are one artifact rather than two copies
that agree until they don't.
test-go.yml gains 'ci/**' and '.gitea/workflows/release.yml' in its paths.
Without that the guard exists but never fires on the changes it protects,
which is the same nothing it replaces.
What is pinned, and why each one:
- HHMM is zero-padded. A build at 00:42 must emit "0042"; a stripped
leading zero shifts the segment two orders of magnitude and reverses
comparisons against every other build that day. It only bites for a
tenth of the day, so it will not be found by chance.
- The name derives from the COMMIT and the code from the BUILD. Asserted
by holding one clock and moving the other: the name must not move, the
code must.
- The code clears 1895, the highest versionCode the retired commit-count
scheme shipped. Below that Android refuses the upgrade as a downgrade
and the channel becomes a one-way door.
- The tag is the name with a `v`, never chosen.
- release.yml still calls the script, and does not derive a commit count
again. This pins the WIRING: without it every other assertion keeps
passing while the shipped path silently drifts out of coverage.
The script rejects unusable clocks rather than emitting something plausible,
and those rejections are tested — a guard that cannot fail is worse than
none, because it reads as coverage.
Falsified before committing rather than after: ran the script against good
and broken inputs and watched all three failure paths fire; verified every
asserted value by executing it rather than by reading it; and checked the
two workflow predicates catch their regressions while staying immune to a
comment that merely names the old formula.
Step 5 of 5 — Scribe task #3812, milestone #390.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
|
||
|
bvandeusenandClaude Opus 5
|
ca1c18bbbb |
fix(android): miniplayer content sat at the top of its bar, not centred
android / Build + lint + test (push) Successful in 4m9s
Reported with a screenshot: the bar drew at full height but the cover, title and transport row hugged its top edge, leaving an empty strip of surface above the gesture area. The Surface is a fixed 80dp. Inside it a plain Column stacked a 4dp progress fill and then MiniRow at its INTRINSIC height — 48dp, set by the cover and the icon buttons. A Column stacks from the top and nothing claimed the remainder, so 80 - 4 - 48 = 28dp collected at the bottom. Measured off the screenshot rather than eyeballed, and the bands agree exactly: progress fill 14px (4dp at 3.5x), surface 280px (80dp), cover 167px (48dp), empty below 99px (28.3dp). That the arithmetic lands on the measurement is what makes this the whole cause rather than one contributor. MiniRow was already centring its content correctly — inside a box that was only ever 48dp tall. Giving it weight(1f) lets it take what the progress fill leaves, so it measures 76dp and centres 48dp of content: 14dp above and below. The fill stays pinned to the top edge, which is where a progress indicator belongs. Not the same bug as issue #2681. That was a dead strip ABOVE the miniplayer from an unclaimed navigation-bar inset, fixed in v2026.08.18. This is inside the bar, pure layout, no insets — the surface already stopped correctly above the gesture area. CI cannot see this one. There are no Compose UI tests in the repo; the Android lane is ktlint, detekt and JVM unit tests, and a layout bug needs an instrumented test to catch. Compilation and lint are all this commit gets from CI — the visual check is on a device, and the APK only builds on a tagged release. Scribe issue #3826. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH |
||
|
bvandeusenandClaude Opus 5
|
68136c64c0 |
fix(android): decide updates on the ordering key, not the version name
android / Build + lint + test (push) Successful in 3m55s
The app compared NAMES while Android installs by versionCode, with nothing keeping the two orderings consistent. So it could offer a build the platform then refused as a downgrade, or stay silent about one it would have accepted. The offer and the install were asking different questions. Both consumers — the shell banner and the About card — now route through one isUpdateAvailable(): decide on the ordering key whenever the server reports one, since that is the same value the package installer compares, so an offer implies an install that will actually be accepted. Name comparison survives only as the fallback for a server predating the field. isVersionNewer is deliberately untouched. It already degrades per segment and is not what was broken; rewriting it while nearby would have put the fallback path at risk for no gain. code is nullable on the wire, and that is load-bearing rather than stylistic. The app's Json sets coerceInputValues = true, which replaces a JSON null with the declared default on a NON-nullable property — so `val code: Long = 0` would have turned "this server reports no ordering key" into "its key is 0" silently, ranking every such server as infinitely behind and offering its build to everyone forever. Reading the field declaration alone would never show that; it lives in AppModule. A third caller turned up during the sweep and was deliberately left alone. NetworkStatusController compares the /healthz minClientVersion, which is a server-declared compatibility floor rather than the bundled APK — there is no ordering key on that wire at all, so names remain the only thing it can compare. Different question, correctly still using the old helper. The update channel had no tests whatsoever before this, which is worth stating: the thing deciding whether anyone is ever offered an update fails silently in both directions. The new suite pins that the key wins when it disagrees with the name, that a null key falls back rather than reading as zero, the recorded migration constraint (a new-scheme name outranks an old-scheme one across a day boundary but NOT within the same day), and the degradation cases — including that an unparseable DECIDING segment reads as zero and loses, which is why the channel must never live inside the name. Every assertion was checked against the real comparison by mirroring it, rather than from reading it: two of my first-draft comments described the wrong mechanism and were corrected on the evidence. Step 4 of 5 — Scribe task #3811, milestone #390. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH |
||
|
bvandeusenandClaude Opus 5
|
9f3e0b8cd3 |
feat(version): sidecar and /api/client/version carry name, code and channel
The client compares names while Android installs by versionCode, and the wire had no way to close that gap: the sidecar was one positional line and the endpoint returned a name only. This is the plumbing that makes the ordering key decidable by the client at all. The sidecar is now JSON rather than a grown positional string. That shape was chosen against a specific failure: the obvious growth path was "<name> <code>", which a first-space split silently mangles the moment a third field appears — the code stops parsing as an integer and the reader falls back to name comparison WITHOUT erroring. JSON cannot mistake a new field for an old one. code is a POINTER on both sides, and omitempty on the wire. Absent has to stay distinguishable from zero: a build published before ordering keys were recorded genuinely has no code, and zero would claim it is infinitely old rather than unknown. A malformed sidecar now fails loudly instead of serving a blank version. If an unreadable file produced an empty name, every client would compare against nothing, conclude it was current, and go quiet — "I cannot read this" and "there is nothing newer" would return the same answer, which is the failure mode nobody reports because nobody is offered anything to report. The non-tag :latest path no longer RECONSTRUCTS the bundled APK's version. android-release now publishes the sidecar as a release asset beside the APK, and the image build downloads it. The old reconstruction duplicated a derivation formula across two files, and could only ever recover the name — the ordering key is build-time minutes and exists nowhere once that build ends. Releases predating the sidecar report their name with a null code, which is the honest answer rather than a guessed one. image-release also drops to a shallow checkout: it needed full history and tags only to re-derive versions from the tagged commit, and now touches git for nothing. MINSTREL_VERSION comes from GITHUB_REF. Two things checked rather than assumed. The Android Json sets ignoreUnknownKeys, so the added fields cannot break already-installed apps. It also sets coerceInputValues, which will silently turn a null code into 0 if step 4 declares the field non-nullable — recorded on task #3811, because reading the field declaration alone would never reveal it. Also fixes a stale comment block describing "the Flutter client", deleted in v2026.08.18. Step 3 of 5 — Scribe task #3810, milestone #390. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH |
||
|
bvandeusenandClaude Opus 5
|
e46c6bcccf |
docs(release): tags become vYYYY.MM.DD.HHMM, and stop telling people to move them
The tag is now the artifact's own version name with a `v` in front, so `v2026.09.10.1432` and `2026.09.10.1432` are one string. Nothing has to reconcile what the tag claims against what the APK reports, and minting one is arithmetic on the tagged commit's timestamp rather than a lookup. The substantive change is the prose. release.yml's header instructed the reader to `git push -f origin vYYYY.MM.DD` on a same-day re-cut. That is the operation the family rulebook forbids outright, and it has incidents behind it — moving a same-day tag forward once took a published release down with it. Anyone who had installed from that tag was holding something it no longer pointed at. With HHMM there is nothing left for mutability to buy: every tag is unique by construction, so a second release the same day is not a collision to resolve, just another tag. The old instruction is recorded as retired rather than deleted. Someone who remembers it should learn it was withdrawn and why, not find it silently absent and assume they misremembered. README contradicted itself inside one sentence — "immutable per-day release tags ... a same-day re-cut moves the tag forward" — and now says which it is, plus a note that pre-2026-09-10 tags keep the old shape and still work. Transition wrinkle, deliberately left for step 3: the non-tag :latest path reconstructs the bundled APK's name from the latest release's commit timestamp, which for the one existing old-shape release yields 2026.09.09.1828 while that APK actually declares 2026.09.09.1895. It fails SAFE — 1828 compares lower, so no false update is offered — and it self-corrects at the first new-scheme release. Step 3 removes the reconstruction entirely by having the sidecar carry recorded values instead of derived ones. Step 2 of 5 — Scribe task #3809, milestone #390. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH |
||
|
bvandeusenandClaude Opus 5
|
bfdaed9365 |
fix(release): derive versionCode from build time, versionName from commit time
android / Build + lint + test (push) Successful in 4m19s
versionCode was `git rev-list --count HEAD`, and build.gradle.kts called it
"monotonic forever". It is not, and that claim was sitting directly above
the bug it denied.
A commit count runs ahead on `dev`. So a dev build carried a HIGHER code
than the `main` release meant to supersede it, and Android refuses that
install as a downgrade — a channel you can enter and cannot leave without
uninstalling and losing local data.
Two clocks now, and the split is deliberate even though it reads like an
inconsistency:
The NAME answers "is this the same code?", so it derives from COMMIT time
and reads identically on every lane building this source. A dev build and
a main build of one commit must report the same string. Build time cannot
do that — it prints two numbers for one thing.
The ORDERING KEY answers "may this be installed over that?", so it must be
monotonic BY CONSTRUCTION: minutes since 2020-01-01. Commit time fails
here for the mirror-image reason — rebuild an older commit and it goes
DOWN, which on a phone is a refused install rather than a confusing label.
The non-tag :latest path reconstructed the bundled APK's name with the old
formula, so it is moved to the same commit-timestamp derivation. That
duplication is temporary: once the tag becomes `v<version-name>` it
collapses to `${TAG#v}` with nothing left to keep in step.
Verified locally by running the derivations rather than reasoning about
them: HEAD yields 2026.09.09.1828; the key yields 3519456 against ~1895
from the old scheme, inside int32 with ~4000 years of headroom; a commit
at 00:42 UTC yields "0042", not "42". The workflow now asserts the emitted
shape too — a malformed name builds, signs and publishes happily and only
surfaces as an update nobody is offered, which nobody reports.
That local check is the only verification this commit gets. release.yml
triggers on main and tags only, so nothing on `dev` executes the new
derivation; CI here proves the Gradle file still parses and nothing else.
Also confirms the migration constraint recorded in milestone #390: this
commit would name a release 2026.09.09.1828, which is LOWER than the
installed 2026.09.09.1895 under name comparison. The first new-scheme
release must be cut on a later calendar day, or existing installs will
never be offered it.
Step 1 of 5 — Scribe task #3808, milestone #390.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
|
||
|
bvandeusenandClaude Opus 5
|
c27f9d484a |
test(android): guard that the typefaces stay bundled
android / Build + lint + test (push) Successful in 5m17s
The web side has no-external-assets.test.ts; Android had nothing, so the font provider could come back with no test noticing. This is the Android half. Expectations are read out of Typography.kt rather than hardcoded, which is what makes it a structural pin instead of a list that rots: the guard extracts every Font(R.font.X, FontWeight.WN) declaration and checks that X.ttf exists, is really TrueType, and reports N as its OS/2 usWeightClass. Add a face without vendoring it and this fails; change a declared weight without refetching the matching static instance and it fails too. usWeightClass is the check worth having. css2 silently collapses a multi-weight request to 400 for legacy clients, so Medium comes back as Regular — a valid TrueType file that renders at the wrong weight everywhere, and the only field that distinguishes it. Comments are stripped before the absence check, so the KDoc explaining why there is no GoogleFont reference cannot satisfy the assertion that forbids it. Falsified by mirroring every predicate and byte offset against the real files: it passes on what is committed, and trips on HEAD~1's Typography.kt via both the forbidden-symbol check and the no-declarations-found check. A 400 file asserted against a declared 500 fails, so the weight comparison is not vacuous. Compilation itself is unverified locally — no Gradle run here — so CI is the first thing to actually build this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH |
||
|
bvandeusenandClaude Opus 5
|
b52a00df66 |
fix(android): bundle the typefaces instead of fetching them at runtime
android / Build + lint + test (push) Successful in 4m19s
Typography.kt resolved Fraunces, Inter and JetBrains Mono through the Play Services font provider, which fetches them over the network on first use. Same rule-164 problem the web client had, with a second failure mode on top: the provider is absent entirely on devices without Play Services, so the app fell back to the platform default and stopped looking like Minstrel — quietly, with no error. The five static instances now live in res/font, vendored by the same tools/vendor-fonts.py that produces the web bundle. Both clients draw from one list of faces so they cannot drift apart. Cost is ~0.86 MB of APK; the runtime path is removed rather than kept as a fallback — the ui-text-google-fonts dependency, its version-catalog entry and the provider certificate hashes in font_certs.xml are all gone. Two things about fetching TTFs that are worth writing down, because both fail by succeeding: Google Fonts picks the format from the User-Agent, and there is no parameter to ask for one. A modern UA gets woff2, which res/font cannot load. The obvious "use an old UA" fix gets EOT — an IE-only format that downloads happily, has a plausible size, and is entirely useless here. An Android 4.4 UA is what actually yields TrueType. css2 also collapses a multi-weight request to 400 for legacy clients, so asking for Medium silently returns Regular: a valid TrueType file that renders at the wrong weight everywhere. Each weight is therefore fetched on its own URL, and the script now asserts OS/2 usWeightClass on every download — that field is the only thing distinguishing the two files. Verified before wiring: all five carry TrueType magic, the 400/500 pairs differ, and their usWeightClass reads 400/400/500/500/400 as declared beside them in the FontFamily. Not covered: there is no guard for this on the Android side. The web equivalent is asserted by no-external-assets.test.ts, but the Android tree has no source-inspection test pattern to follow and no way to falsify one without a local Gradle run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH |
||
|
bvandeusenandClaude Opus 5
|
16005054eb |
fix(web): vendor the web fonts instead of loading them from Google
test-web / test (push) Successful in 42s
app.html linked its stylesheet straight from fonts.googleapis.com, with preconnects to that host and fonts.gstatic.com. A deployed instance has no outbound network, so those requests never arrive and the whole UI renders in fallback faces — Georgia for the display face, whatever the system has for Inter and JetBrains Mono. This is invisible in development, which is why it survived: the dev machine has internet, so the fonts load and everything looks right. Only a real deployment shows the failure. tools/vendor-fonts.py fetches the three families once and writes them under web/static/fonts with a generated stylesheet. static/ is copied into the SvelteKit build, which Go embeds, so the faces travel inside the binary. 32 woff2 files, 912K. Two details that matter for correctness rather than size: Urls in the generated CSS are relative (./Inter-400-latin.woff2), not absolute. A url() resolves against the stylesheet's own address, so the directory keeps working when the app is served under a base path; /fonts/... would not. Every subset Google slices is kept, with unicode-range intact. The browser still fetches only the ranges a page uses, so this costs repository bytes rather than request bytes — and a library full of Cyrillic or Greek artist names renders instead of falling back mid-list. The guard asserts the property, not the vendor: any absolute url in a resource-loading attribute fails, whoever hosts it, since naming Google would pass the day someone reached for a different CDN. It also checks preconnect separately (those carry no fetch of their own, so the url check misses them), strips HTML comments before asserting an absence so prose describing the forbidden thing cannot satisfy the check, and pins the font families to tokens.json rather than a hardcoded list. Falsified against the pre-change app.html: it trips both the external-url and preconnect assertions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH |
||
|
bvandeusenandClaude Opus 5
|
b06a1adfe8 |
feat(brand): draw a reduced mark so the favicon reads at 16px
The full hat does not resolve below ~32px, which the header worked around by sizing up. A browser tab cannot: it renders the favicon at 16px and does not ask. There the mark was a blob. Deriving a small form from the traced art does not work, and this is the non-obvious part. Hole-filling, morphological smoothing and dropping components were all tried; every one of them preserves the overall silhouette, and the overall silhouette — dominated by a long diagonal plume — is precisely what fails. The result each time was a diagonal smear that reads as no object at all. So the reduced form is drawn rather than derived: a strong horizontal brim under a crown that peaks left of centre, a band slit so the two do not fuse, and a short pointed plume. Same lean and proportions as the full mark, detail removed instead of minified. favicon.svg and favicon.png now use it; apple-touch, icon-512 and the Android launcher icons keep the full art, being large enough for it. The plume carries the accent, which measures 3.04:1 on obsidian and 5.43:1 on the light ground — both clear of the 3:1 graphics floor. Also corrects the accent-on-iron figure in the generator's comment from 2.80:1 to 2.70:1. The real --fs-iron is #1E2228; 2.80 came from measuring against a value I had guessed rather than read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH |
||
|
bvandeusenandClaude Opus 5
|
5593f7ce17 |
feat(brand): replace the M mark with the traced bard-hat logo
The mark is now a feathered hat with an arc of eighth notes, traced from the operator's reference artwork at 99.74% IoU. The hat takes the text colour and the note arc holds the accent — the same construction the M used, and for the same reason: parchment on a light surface is invisible, so the silhouette has to flip with its background while the accent stays constant. This reverses the subject-neutrality argument recorded in Minstrel's design system, which held that depicting a bard would tell a new user the app is for renaissance-faire music and had twice rejected a hat. The operator commissioned this artwork and chose it with that objection on the table; the record is updated rather than silently contradicted. Both accent-filled alternatives were measured and rejected: #4A6B5C is 3.04:1 on obsidian and 2.80:1 on the raised iron, so an accent hat drops under the 3:1 graphics floor as soon as it sits on a card. tools/gen-brand-assets.py is the single source for the four copies, which cannot share a file because each needs a different colour mechanism — currentColor inlined, a prefers-color-scheme swap in the favicon, literal fills in mark.svg, flat pixels in the rasters. Hand-copying 20KB of path data four ways is how a silhouette change lands in three of them. Two notes on the trace, both non-obvious: it runs on the original antialiased greyscale rather than a binary mask, because tracing a supersampled mask scores ~100% IoU by reproducing the pixel staircase exactly — a perfect number for jagged art at 120KB of path, versus 99.74% at 20KB. And potrace reads PBM where bit 1 is black, so the ink mask is inverted going in; backwards, it traces the background and still emits a plausible-looking SVG. The header lockup moves 20px → 28px: the hat carries far more detail than the M and does not resolve below ~32px. The 16px browser-tab favicon is still a blob at that size and is not addressed here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH |
||
|
bvandeusen
|
0103953953 |
refactor(diagnostics): split the flap window and episode rule apart
android / Build + lint + test (push) Successful in 4m8s
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. |
||
|
bvandeusen
|
72c0e96f92 |
fix(player): stop the local player during a cast; capture transport flap
android / Build + lint + test (push) Failing after 1m11s
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. |
||
|
bvandeusen
|
e87516bbe4 |
refactor(player): split Sonos queue loading out of the picker — #2728
android / Build + lint + test (push) Successful in 3m48s
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. |
||
|
bvandeusen
|
8e21bce103 |
fix(player): verify the Sonos queue actually landed — #2728
android / Build + lint + test (push) Failing after 1m18s
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. |
||
|
bvandeusen
|
955a61194e |
fix(library): a fully-missing album leaves the year axis too — #2702
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
|
b96285d6d9 |
test(android): TrackRef needs albumId and artistId — #2704
android / Build + lint + test (push) Successful in 3m46s
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. |
||
|
bvandeusen
|
7ba673ed83 |
fix(library): tell clients when a file goes missing or comes back — #2704
The wire field shipped in
|
||
|
bvandeusen
|
366692a1fc |
fix: stop the sync feed hiding missing files from clients — #2704
#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. |
||
|
bvandeusen
|
6d729d1512 |
fix(web): timeUntil rounds, so a 4h wait doesn't read as 3h — #2527
test-web / test (push) Successful in 34s
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. |
||
|
bvandeusen
|
414dfb23b6 |
feat: show what re-acquisition has done, per folder — #2527
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). |
||
|
bvandeusen
|
952132714e |
feat(web): re-acquisition settings card on the missing-files page — #2527
test-web / test (push) Successful in 34s
Rule #27: the sweeper has been running since
|
||
|
bvandeusen
|
c2862e97bd |
test(api): cover the missing-file admin routes in the Mount test — #2527
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.
|
||
|
bvandeusen
|
30a5ac56ce |
feat(api): admin endpoints for the re-acquisition policy — #2527
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. |
||
|
bvandeusen
|
bab9b16831 |
feat(library): a missing file asks Lidarr for itself, on a backoff — #2527
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. |
||
|
bvandeusen
|
03a8d12079 |
docs: stop pointing at the deleted Flutter tree — #2710
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. |
||
|
bvandeusen
|
0036f534db |
chore: delete the Flutter client — #2710
android / Build + lint + test (push) Successful in 4m1s
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. |
||
|
bvandeusen
|
bfb6c9acfe |
style(android): satisfy detekt on the new browse tabs — #2467
android / Build + lint + test (push) Successful in 3m54s
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. |
||
|
bvandeusen
|
3eada70aac |
feat(android): Genres and Years browse axes in the Library — #2467
android / Build + lint + test (push) Failing after 1m22s
#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. |
||
|
bvandeusen
|
d9238ec5be |
fix(android): notice when a Sonos stops on its own, and get it going again — #2700
android / Build + lint + test (push) Successful in 3m57s
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.
|
||
|
bvandeusen
|
a31b672b14 |
test(web): admin nav is nine tabs — #2527
test-web / test (push) Successful in 40s
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. |
||
|
bvandeusen
|
8d1f2674fd |
feat(web): admin page for files the library has lost — #2527
test-web / test (push) Failing after 32s
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. |
||
|
bvandeusen
|
845f45fb0b |
refactor(web): one relativeTime for the triage surfaces — #2527
test-web / test (push) Successful in 40s
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.
|
||
|
bvandeusen
|
aab90a7a39 |
feat(android): name the missing file behind a greyed playlist row — #2527
android / Build + lint + test (push) Successful in 3m42s
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
|
||
|
bvandeusen
|
4c49ee2cc6 |
feat(web): a playlist entry whose file is missing greys out and is skipped — #2527
test-web / test (push) Successful in 33s
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. |
||
|
bvandeusen
|
4dd0a58d63 |
feat(api): admin surface for files the library has lost — #2527
The scan has marked missing files since
|
||
|
bvandeusen
|
c3f3a17c6d |
feat(library): a missing file stays in the playlist, greyed and unplayable — #2527
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
|
||
|
bvandeusen
|
20bd7bfaf8 |
fix(android): let list content reach the MiniPlayer — #2681
android / Build + lint + test (push) Successful in 4m28s
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. |
||
|
bvandeusen
|
8e1d25a772 |
fix(scanner): repair acronym and apostrophe casing on genre tags — #2468
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. |
||
|
bvandeusen
|
4509f740f8 |
feat(web): sort the genre index A–Z as well as by count — #2468
test-web / test (push) Successful in 38s
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. |
||
|
bvandeusen
|
a254cb2273 |
ci(release): close the verify blind spot, check preconditions before the build
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. |
||
|
bvandeusen
|
e368b82f0a |
ci(release): fail loudly when a tag release ends up without its APK
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. |
||
|
bvandeusen
|
304de88c50 |
test(tuning): assert headers by exact accessible name — #2495
test-web / test (push) Successful in 34s
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. |
||
|
bvandeusen
|
96abb48086 |
test(tuning): query the window/last-week headers as column headers — #2495
test-web / test (push) Failing after 33s
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. |
||
|
bvandeusen
|
a094d5f8b0 |
test(metrics): target deltas by test id, not by glyph — #2495
test-web / test (push) Failing after 35s
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.
|
||
|
bvandeusen
|
481f906059 |
feat(metrics): publish margin of error on every delta — #2495, #2524
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. |
||
|
bvandeusen
|
24d330424f |
feat(library): adopt moved files instead of forking their history — #2528
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. |
||
|
bvandeusen
|
f6d1cf24f0 |
feat(library): detect missing files and stop offering them — #2523
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. |
||
|
bvandeusen
|
fd27819cdd | style(scanner): tagged switch on ID3 major version — #2499 |