357 Commits

Include renames

Author	SHA1	Message	Date
logan	49445ded77	test(download): write the yt-dlp stub under ForkLock ... The kernel refuses to exec a file that is open for writing anywhere in the process, and these tests are parallel: a sibling's fork duplicates stubYtDlp's write descriptor in the moment it is open and carries it past our close, so the exec a moment later fails with ETXTBSY. That is the flake seen once locally and once in CI, both times on a tree with no Go in its diff. Closing sooner is not available -- os.WriteFile has already closed the file before anything execs it -- and O_CLOEXEC does not help, because the window is between another goroutine's fork and its own exec. syscall.ForkLock is the lock forkExec takes across that fork, so holding it over the write means no child can exist while the descriptor does. Measured on the helper itself under 12 concurrent writers: 176-189 of 2400 execs refused before, 0 of 2400 after, three runs each. Closes #146	2026-08-30 07:40:32 -04:00
logan	a113b7bd62	fix(riff): grow a chunk buffer with what arrives ... Parse sized its buffer from the chunk header, which is four bytes read off the file, so a truncated or malformed WAV declaring a 4 GB data chunk in a 2 kB file got 4 GB from the allocator before the read discovered there was nothing to put in it. The error was always right; the allocation happened first. io.CopyN into a bytes.Buffer is what ID3Chunk beside it has done since #104, and needs nothing new: the reader stays an io.Reader and the buffer grows with what actually arrives. The regression test measures rather than asserts the error, because the error is identical on a build that allocates the gigabyte. Measured on the pre-fix build: 1,073,750,920 bytes of TotalAlloc for a 42-byte container whose data chunk claimed 1 GiB. Closes #216	2026-08-26 06:35:04 -04:00
logan	c56eae2959	fix(metadata): read a WAV's tags out of its RIFF id3 chunk ... tagwriter has always written a WAV's tags into a RIFF "id3 " chunk correctly, and dhowden/tag -- which metadata.ExtractTags is built on -- has no RIFF reader at all. So the app could not see tags it had just written: editing tags on a WAV, autotagging a WAV folder or importing a WAV download all appeared to succeed and changed nothing the library could show, while the file on disk really was tagged and other players read it. backend/riff is a new package rather than a move into either half, because tagwriter already imports metadata: reaching back for parseRIFF is an import cycle, not merely the wrong direction. backend/tagtotals is the precedent. Its two readers are deliberately different. Parse holds every chunk in memory, which is what rewriting a file needs -- and a WAV's audio *is* a chunk, so doing that on the scan path would read every WAV in the library in full. ID3Chunk seeks over what it is not looking for. The container is asked before tag.ReadFrom rather than after it fails, because that library's last resort is an ID3v1 trailer and a WAV carrying both would otherwise be read by the wrong one. An untagged WAV -- no chunk, an RF64 container, a tag with every frame cleared -- reads as empty metadata with no TagReadWarning: the scanner's filename fallback is the right answer there, and a warning would report a fault on a healthy file. The gap was pinned by TestWAVTagsAreNotReadableYet, which failed the moment the reader learned and said in its own comment what to update. So it goes, TestFixturesMatchManifest no longer skips wav, and totals_test.go's WAV case reads through metadata.ExtractTags like the other three formats -- a round trip asserted through the writer's own parser was a test of the writer, which is why nothing caught this. Closes #104	2026-08-24 05:44:39 -04:00
logan	30c6b665f1	fix(system): give the process a temp directory that exists ... Android has no /tmp and hands an app no TMPDIR. Go's os.TempDir() falls back to "/tmp" when the variable is unset, so every library in this process that wants scratch space was being handed a path that has never existed. SQLite is the one that noticed, and it said so precisely: W/yellowjacket: msg="champion index rebuild failed" explore.search-index.error="populate champion fts: disk I/O error (6410)" 6410 is not a generic I/O error. `6410 & 0xff` is 10, SQLITE_IOERR, and `6410 >> 8` is 25 -- SQLITE_IOERR_GETTEMPPATH. SQLite could not work out where to put a temporary file. Two measurements on the device say why: `ls -d /tmp` does not exist, and the app process's environment carries no TMPDIR. A shell's does (/data/local/tmp), which is why this is easy to miss from `adb shell`. The cost was a silent performance cliff on the slowest device this app runs on: `championReady` stayed false, so every Explore search took the generic path over the whole 1,079,667-row index instead of the champion subset, and the rebuild was re-attempted on every launch. **The class is fixed rather than the statement.** The trigger is the *size* of the work, not that query -- anything that spills fails the same way there, so large sorts, large joins and VACUUM were all waiting their turn. The repair belongs at the process's one answer to "where do temporary files go". `PRAGMA temp_store = MEMORY` was the alternative: cheaper, more local, and a promise that every future spill fits in RAM on a phone. The catalog is the largest thing in this app and that is not a promise worth making silently. UseTempDir sits beside UseHomeOverride and carries its two rules for the same reasons. **An empty base is a no-op**, because that is what application.Mobile.StoragePath() returns on desktop -- so this needs no build tag and changes nothing off mobile, where /tmp is real. And **an explicit TMPDIR wins**, so anyone who set one deliberately gets it; nothing sets it on the platform this exists for. It needs no new Wails API and no Java change: StoragePath() is already what YJ_HOME is pointed at, and the directory goes under it. Two things beyond the rename of a variable. **Writability is probed, not assumed.** MkdirAll on an existing unwritable directory succeeds, so without the probe this could set TMPDIR to a directory nothing can use -- which is the same bug one directory over, and just as quiet. **It returns its error, and main logs it.** A temp directory that could not be created is the same silent failure one step earlier. A failure is not fatal: it leaves the platform's answer in place, which is what every release before this one ran with. That log line is readable on the platform only because of #160. Verified on the reference device, where the same launch that used to print the failure now prints: I/yellowjacket: msg="champion index rebuilt" explore.search-index.elapsed=6.496s Closes #190	2026-08-21 17:23:23 -04:00
logan	d034d6e571	fix(explore): resolve pending release MBIDs against the real table ... The release-group MBID backfill queried `release_groups`, which plan 013 renamed to `albums`. It failed on its first statement on every launch since e7748f1 and the pass returned quietly having done nothing: W/yellowjacket: msg="release-group mbid backfill: query failed" explore.error="SQL logic error: no such table: release_groups (1)" What it does is resolve a release-level MBID (MUSICBRAINZ_ALBUMID, which many taggers write instead of MUSICBRAINZ_RELEASEGROUPID) into the release-group MBID everything else on the album page is keyed by. A scan cannot afford a live MusicBrainz call, so `library.updateMBIDs` stashes the release MBID in `pending_release_mbid` and defers to this. With this broken the marker was written by every scan and resolved by nothing, so those albums were untagged as far as the catalog is concerned, permanently. **The fix is to call the queries plan 013 already wrote.** `GetAlbumsWithPendingReleaseMBID` and `ResolveAlbumPendingReleaseMBID` have been in sql/queries/albums.sql since that change, generated and never called -- the writer of the marker was repointed at `albums` and the reader was not. So this is not a missed rename so much as a call site left behind, and thirty lines of raw SQL and hand-rolled scanning become three. That is also the durable half. These two were the last raw-SQL references to a schema table in the tree, and being raw is exactly why 013 missed them: sqlc reads sql/schemas/ and cannot generate against a table that is not declared, which is what made every other statement in the repo immune to the same rename. Three smaller things. **The LIMIT came back.** The raw statement bounded a run at releaseGroupMBIDBackfillMaxPerRun and 013's sqlc replacement had no LIMIT at all, so switching over as-written would have swapped a dead pass for an unbounded one -- each row is a live MusicBrainz lookup on a 1 req/s limiter shared with every page the user can open. **The UPDATE goes through the writer.** `ReadQueries` is a query-only pool and an UPDATE issued on it fails at runtime with "attempt to write a readonly database". **The query is its own method so its failure is assertable.** A test of the pass as a whole cannot see this bug, because a query error and an empty library are the same early return -- which is the whole reason it survived. `pendingReleaseMBIDs` returns the error, and the test reproduces the device's exact message against the old statement. Verified on the reference device: the warning is gone from logcat. Closes #189	2026-08-21 17:23:05 -04:00
logan	842fe47e9e	feat(player): count what the ring buffer misses ... An underrun is audible and nothing counted it. When the ring is empty BufferedStreamer.Stream zeroes the caller's buffer and returns ok, so a run of zeros is spliced into the waveform and the step discontinuity at each edge is a click; a series of short ones is static. That is the one candidate in #135 whose audible signature matches the report. `starved` and `starvedSince` already existed, from #122's stall fix, and could not answer this: they are a *stall* detector, reset by every arriving sample, because their job is to end a track whose source has died and a merely slow source must not be cut short. That reset is exactly what made the audible case invisible -- a hundred 20ms underruns a minute never approach the give-up threshold, so they were invisible to the log, to the UI and to every tier. UnderrunStats counts runs, calls and samples for the life of the streamer. Runs is the number that means something audible: one episode is one pop however many callbacks it spans, and the ratio of runs to calls is what separates clicking from dropping out. Three things about the reporting are deliberate. **Counting is in Stream and reporting is not.** Stream runs on the speaker callback's real-time deadline, so a log line there would allocate, format and write on the exact path whose missed deadline is the defect -- measuring by making it worse. The count is two increments under a lock that was already held; the report is on the 1 Hz position ticker, which only runs while playing. **An unchanged count is not logged**, which is emitStatus' rule one package over. A healthy player is silent, so anything in the log is news and the line appears exactly while it is popping. **It is Info rather than Debug.** The default level is Info and a phone has no convenient way to set YJ_LOG_LEVEL, so a debug line here would be a counter nobody on the affected platform could read -- which is the shape of the bug that made #160 necessary. underrunDelta clamps at zero because the counter belongs to the streamer and the streamer is replaced on every track: a baseline carried across that boundary is the previous track's total subtracted from a fresh zero. The baseline is reset at the load as well; a negative count in a log line reads as a broken instrument and would discredit the measurement this exists to make. This is the instrument, not a fix. What it measures is on the issue.	2026-08-21 16:30:32 -04:00
logan	168e588387	feat(android): route slog to logcat ... Every slog line the app wrote on Android went to /dev/null, including the one naming the error it was about to os.Exit on. #52 is what that cost: a process that vanished with no tombstone, no AndroidRuntime stack and nothing in `logcat -b crash`, at Priority/Critical for months, whose entire diagnosis was one sLogger.Error main.go was already writing. backend/androidlog is a slog.Handler over __android_log_write, chosen in main() by build tag rather than by a runtime check so that a desktop binary links no cgo for a platform it cannot run on. **Everything except the write itself is untagged.** That is androidpayload.go's discipline pushed as far as it goes: the only toolchain that compiles the android tag is a cross-compiler and the only thing that runs it is a phone, so the priority mapping, the formatting, the chunking and the handler's own attr and group bookkeeping are ordinary Go that `go test` exercises everywhere, and android.go is fifteen lines that hand a string to liblog. Four things in it are load-bearing. **The tag is a fixed string, not the application id.** The debug build carries `applicationIdSuffix ".dev"` so it can be installed beside the release app, and it is the only build whose WebView can be inspected -- so a tag derived from the id is a different tag on the one build anybody debugging this app is running, and the filter meant to show these lines would hide them exactly where they were being looked for. **The priorities are android/log.h's own values, asserted twice.** android.go carries constant expressions that do not compile as uint if the header renumbers; the untagged test writes the six numbers out longhand, because comparing a constant to itself passes on any renumbering. A wrong priority is the failure that hides rather than breaks -- logcat prints whatever number it is handed, so an Error filed as Info is present, correct, and invisible to every filter. **Formatting is delegated to slog's TextHandler.** WithAttrs and WithGroup are the half of slog.Handler that is easy to get subtly wrong, and a logger whose groups are wrong is a logger nobody reads. The derived handlers share the parent's buffer *and its mutex*: a second mutex would guard nothing, and two loggers derived from one would splice their bytes into a single line under load. **A line is chunked, because liblog drops what does not fit.** The kernel logger's entry is 4068 bytes for tag and message together and the remainder goes without comment, so a long record would be truncated in the middle of the thing worth reading. Time and level are dropped from the formatted line, since logcat stamps every entry with both -- and dropping them by *key* also ate a caller's own "level" attribute, which the on-device probe caught and TestACallersOwnLevelAttrSurvives now holds. ReplaceAttr sees an empty group path for the built-ins and for every top-level attribute alike, so the kinds are what separate them. Verified on the reference device (TLP301, Android 14): a debug build logs I/W/E under the `yellowjacket` tag at the right priorities, and the first thing it surfaced was a real warning nobody could previously see -- `champion index rebuild failed ... disk I/O error (6410)`. Closes #160	2026-08-21 16:30:32 -04:00
logan	867ced8c81	feat(player): leave the volume to the system where the system owns it ... On Android the hardware keys are the volume control and the framework mixes our stream against the device level, so a second control inside the app moves something the user already moved. Where that is true the player's level sits at maximum, SetVolume / ChangeVolume / MuteToggle are refused, and nothing persists a level nobody chose: restore remembers the stored value instead of applying it, and saveState writes that same value back rather than recording the synthetic maximum. Mute is in that list because it is a level of zero by another name -- and because with no control rendered it would be the one state on such a platform the user could not get out of. The predicate is named after the capability rather than the platform, because that is what makes it testable. Only platformOwnsVolume is behind a build tag, in two files that declare nothing else; everything else is decided against Player.systemVolume, a field a test sets either way. That is mediacontrols' split, with androidpayload.go's reasoning for keeping the contract out of a tagged file, and the tagged pair is covered by a source sweep since no tier here compiles both halves. SetDuck is deliberately untouched: it applies its attenuation by re-applying the *user's* level through setVolumeLocked, so pinning that level to maximum leaves the offset arithmetic exactly as it was. It is the only thing that may still move the output on such a platform, and TestSystemVolumeStillDucks is that property rather than a comment.	2026-08-21 00:53:21 -04:00
logan	dc8db159f9	feat(player): centre the transport and show the volume inline ... Two issues over one bar, because they are one relayout. #42's own findings say so: giving wa-slider a label grows it 6px to 14px and moves the transport, which is #23's subject, so doing them in sequence means measuring the bar twice and throwing the first set away. The bar was `320px 1fr auto`, so the transport sat in the middle of what the metadata and the queue button did not use — its centre was ~140px right of the window's at every width. The outer two tracks are the same expression now, so the middle is centred by construction. The side width is the metadata's, capped at a quarter of the bar, and the cap was measured as a regression before it was a decision: reserving the full `--now-playing-width` on both sides is perfectly centred and takes the seek bar's track from 257px to 61px at 800px, and to 0 at 200% text. The control you drag was paying for the symmetry. With the cap it is 246, which is parity. It is a `min()` rather than a breakpoint because that variable is user state — the metadata has a drag handle — and tying both sides to it is also what keeps dragging meaningful; a plain `1fr … 1fr` centres just as well and silently makes the handle a no-op. The volume moved out of `audio-player` into the bar because the transport column has to hold the transport and nothing else, and it joins the queue button in one cell rather than a second column, since the centring compares columns. It is a slider by default and a popup by setting. The stored flag names the *popup*, which is this config's polarity rule — the zero value has to be the intended answer, so an existing config.toml gets the new default with no migration. Inline, the icon is the mute toggle and is named after that action rather than the state, because with the slider beside it there is nothing to disclose; the component tier now covers both presentations rather than whichever is default. Three nested rules in this block began with a bare element selector, which Chrome 120 relaxed and the phone's Chrome 113 **silently drops** — including the ellipsis on the bar's own title and artist, which has therefore never truncated on the device. They are `&`-prefixed now. Filed as #154 for the class and for a check. `bottom-bar.spec.ts` pins both halves separately on purpose: an uncapped build is perfectly centred and fails only the seek-bar width, so a spec asserting centring alone would have passed the regression above. Both were verified by mutation. Closes #23 Closes #42	2026-08-20 00:40:52 -04:00
logan	8efed2dd2b	refactor(shell): retire the Jobs destination ... Nothing it carried is gone -- the two commits before this put all of it somewhere the work is already being done. A retired destination is the one shape #25's storage decision does not make free. A visibility entry is a map key and an unknown key is dropped on load; a launch page is a *value*, and an unknown one fails validation -- which on the load path means the app refuses to start for whoever had Jobs selected. `RetiredViews` is that list, read by `ApplyDefaults`, which treats a retired name as a zero value. An unknown-but-not-retired name still errors, because that is a typo and saying so is the useful answer.	2026-08-19 20:27:40 -04:00
logan	a3926704cc	feat(config): make the shell's destinations configurable ... The sidebar's eleven entries are more than most libraries need, and Autotag rewrites tags on disk, which is not what a fresh install should be one click from. Stored as a map keyed by view id, where an absent key means that view's own default. A `HiddenViews []string` cannot express "Autotag off by default" -- its zero value is *hide nothing* -- and a boolean per view turns a view that later stops existing into stored garbage. With a map, an unknown key is dropped on load, a view added later gets its own default, and no install needs migrating in either direction. Same polarity as AllowMeteredCatalogDownload: the zero value is the intended answer. `Views` is also what DefaultPage now validates against, so which views exist and which may be the launch page are one list rather than two. Two states the user could not get out of are refused rather than allowed: Settings is never hideable, and the launch page is not hideable while it is the launch page. Both refuse in the *config*, not in the UI, because `config.toml` is hand-editable. On load the launch page is instead un-hidden -- there is nobody to tell, and the honest reading of "my launch page is Autotag" is that this user wants Autotag, not that their launch page should be silently reset.	2026-08-19 19:33:32 -04:00
logan	018d857746	feat(shell): global back and forward in the top bar ... The history stack has been global since the Android back gesture landed -- every navigation is an entry and `popstate` restores any of them in either direction. What the report describes as "back is tab-scoped" is that the only way back was a detail view's own button, which leaves the screen with the view it belongs to: click over to Tracks and the album you were reading is still one entry away with nothing on screen saying so. `<nav-history>` is that affordance, plus `nav.back` / `nav.forward` on Alt+Left / Alt+Right -- the browser's own combination, and clear of the bare arrows that seek, since a binding matches on its full canonical string. Forward is not back negated, which is why the old `pushedEntries` counter is gone rather than extended: `popstate` carries no direction and fires identically both ways, so one counter decremented on every pop reads a forward as a second back. Each entry carries its index and the shell keeps the current one and a high-water mark, which also survives a jump of more than one. The buttons dispatch the events the rest of the app already dispatches rather than calling `history` themselves -- the shell owns the guard that stops a press at the root leaving the app, and a second caller reaching for history is how the old `navStack` came to disagree with the platform. Below 900px the control stands down: the top bar is what runs out of room first below that, and nothing becomes unreachable -- the shortcuts are global at every width and the phone has the platform's gesture. Closes #6	2026-08-19 18:08:55 -04:00
yonlu	23f3d4b3b0	fix(explore): clear in_library on a row that has no local id ... `in_library = 1 AND local_*_id IS NULL` was a fixed point. upsertBatch's conflict clause is `MAX(in_library, excluded.in_library)`, so it can only ever raise the flag, and pruneStaleLocalCrossReferences — which its own comment calls the only place a removal from the library is reflected back into the index — was gated on the id being present. So nothing in the app could clear such a row, ever: a permanent claim of ownership with no local row to check it against. The gate is now the flag *or* the id, for all three entity types. A NULL id fails the existence test on its own, so this needs no second clause to say what "not owned" means. Nothing in the tree writes that shape today — collectLibraryEntities sets both together — which is why this is worth closing rather than leaving: the exposure is a database written by a version whose local-id columns were populated differently, and the next writer that sets the flag without an id, which nothing structurally prevents and which this shape made permanent rather than merely wrong until the next scan. The test seeds the row with raw SQL on purpose. upsertBatch writes a zero LocalArtistID as literal 0, and 0 satisfies `IS NOT NULL`, so the old gate already caught that shape — a fixture built through the upsert cannot reproduce this at all. NULL is what the artifact importer and any older writer leave behind, the columns being nullable with no default. Reverted against the old gate, it fails on all three types. Closes #118	2026-08-19 14:08:06 -04:00
logan	ede183d026	test(shell): check 900x600, which is narrower than the minimum ... The sidebar collapses to icons *below* 900, so the main panel is 843px at 899 and 700px at 900: the narrowest content area any desktop width produces is at the top of the Compact band, not at the enforced floor. A viewport list that stopped at "the minimum" was missing its own worst case. MinWidth's comment loses both reasons it used to give, because neither mechanism can happen any more — the subtitle is display:none from 899 down, and the sidebar host is overflow-y:auto (at 600x460 its scrollHeight is 434 against a 332px client, and Settings is reachable after scrolling). The value does not change: 800x600 is where desktop chrome stops being comfortable, not where the app breaks, and below 600 the phone layout takes over. A floor defended by two expired mechanisms is a number nobody can argue with, which is worse than either answer. Closes #24	2026-08-19 10:54:52 -04:00
yonlu	2b84bc53e9	fix(player): stop reporting one track's state against another ... Five faults found while auditing the play/pause and position path for a desktop report of the pause icon showing over a seek bar that was not moving. They are one commit because they are one file's worth of tangled state, and two of them do not compile apart. The finished callback did not know which chain it came from. It is dispatched as a goroutine from the beep callback and then queues for p.mu, so a user pressing Next in the last second of a track had it wake up holding the lock for a player that had loaded something else -- and rewind it, stop it, and hand a stale finish to the queue's auto-advance. updateStreamers now stamps a chainID and the callback carries the one it was registered with. (#123) It also emitted PlaybackFinished and PlaybackStateChanged(stopped) *after* releasing p.mu, alone in this file, so a Play() taking the lock in that gap emitted `playing` first and the stale `stopped` landed last -- the button showing play over a track that was audibly running. Both emits are back under the lock. (#123) A source that failed mid-track was reported to the queue as a natural end, so a broken file auto-advanced in silence and was counted as played. The handler takes the reason now: the player cannot name the track, because the metadata is the queue's, so the queue emits PlaybackFailed and skips recording the play. (#123) p.format was assigned once, in the constructor, to the *speaker's* rate, and never again -- so it claimed 44.1 kHz for every file. The replay-after-finish path resamples from it, meaning a finished track played a second time was resampled from a rate the decoder never produced: audibly wrong speed and pitch, and the length and position fallbacks wrong with it. The fixtures are 22050 Hz, which is what lets a test see this at all. (#124) p.trackLengthMs was written only when the database had a row and cleared only by UnloadTrack, so a file with no row inherited the previous track's duration -- and every position report is scaled by it, so the bar reported one track's progress on another's scale. (#125) Queue.OnPlaybackFinished indexed q.tracks[currentIndex] having checked only that the queue was non-empty. currentIndex is -1 whenever the queue has been exhausted, and onQueueExhausted deliberately leaves the finished track loaded -- so playing it from there and letting it end panicked, on a goroutine with no caller to recover it. (#126) The position readers guarded the decoder with the speaker lock, which the read-ahead goroutine has no reason to hold and never takes -- so Position() raced readAhead's Stream() on every position emit, once a second for the whole of playback. srcMu is the lock that excludes that goroutine, and taking it naively deadlocks, because seekLocked already holds it and then emits the landing position from inside that region. seekSourceLocked is that region extracted, so the lock is released before anything is emitted. Found by the race detector, via the test added here for the chain guard: the existing suite never loads a file outside the integration guard, so make test was green over it. (#127) OnPlaybackFinished picks up //wails:ignore along with its error parameter: v3's generator segfaults on a bound method taking an error, and this was never IPC. That removes a binding the frontend could have called to force an auto-advance. Closes #123 Closes #124 Closes #125 Closes #126 Closes #127	2026-08-19 09:06:43 -04:00
yonlu	282dab43eb	fix(player): end the stream when the audio source stops producing ... BufferedStreamer.Stream treated an empty ring buffer as a momentary underrun and answered with silence and ok. That is right while the read-ahead is still going to deliver something, and two of its three exit paths left it never going to: a Close, and a source returning (0, true) in a loop. Neither set done, so the ring drained and every call after it was silence claiming to be audio, for the life of the process. Nothing above this type could tell that from healthy playback. The beep.Seq chain never ended, so the player stayed in Playing with the button showing pause; the decoder's position never moved, so the 1 Hz report pinned the seek bar at a constant -- and since every report resets the bar's interpolation, the report actively suppressed the one thing that would still have moved it. A frozen bar over a track that was not playing, with no watchdog anywhere to notice. Every exit now marks the stream done, and the silence fill is bounded by a duration *and* a run of calls. It needs both. Wall clock is the real measure, because the speaker paces itself and a stall is a question about time -- but a caller draining in a tight loop makes hundreds of calls in microseconds and would outrun a duration alone. A call count alone is the opposite failure, and not a hypothetical one: the first attempt used one and spent the whole budget before the read-ahead goroutine had been scheduled once, ending a perfectly good stream at sample zero and breaking TestBufferedStreamer_BasicStream. Err is plumbed out at the same time, because a drained source and a failed one both arrive as (0, false) and are not the same event. Reading it is a separate change; without it there is nothing to read. Closes #122	2026-08-19 09:05:55 -04:00
logan	9118c16fe3	feat(autotag): answer whether an album has a confident match ... `MatchForAlbum(albumID)` is the question the album detail page needs to ask on open: does the autotagger already have something confident to say about this album, and what would applying it do. **It costs no MusicBrainz request.** Everything it needs is on disk — `tagging_items` carries the top score and release from the background prefetch, `tagging_candidates` durably holds the scored list. The rate limiters here are shared with every page the user can open, so a lookup that fires on page load must not join that queue; a folder nobody has scored yet answers "nothing", rather than scoring it now. **The tier is computed, not read.** `tagging_items.score` is the raw number and `Recommend` is what turns it into a claim, capping it for an ambiguous runner-up, an incomplete alignment or a folder too small to corroborate itself. Filtering on the stored score would promise confidence the scorer had explicitly withheld — which the two-track test pins. **Nothing is said about an album the user has already answered for.** Only a `pending` group qualifies: `confirmed` covers both a finished apply and an explicit "leave as is", and arguing with the second would be actively wrong. The join is `audio_files.group_key`, not a key derived from the folder path, because a group carved out of a mixed-bag folder is keyed on its tags — so a path-derived key would find nothing for exactly the messiest libraries this helps. `GroupCount` is returned because a multi-disc album is one group per disc: a caller that applied to "the album" from a single button would retag one disc of three.	2026-08-19 02:34:11 -04:00
logan	fe67849e57	feat(autotag): name the confidence tier two features have to share ... `ConfidentTier` and `Confident()` are a name for what was about to be written as `== RecommendationStrong` at two call sites: the album page telling the user unprompted that there is a match for what they are looking at (#28), and strict auto-accept rewriting files without asking (#90). A page that claims confidence the auto-accept pass would decline is the app contradicting itself, and #90 asks for exactly this — that the two agree on what "high confidence" means rather than computing it twice. What they do not share is written down beside it. Surfacing a match is a suggestion with a confirm dialog behind it; auto-accept is an irreversible on-disk rewrite gated on further conditions the tier cannot express — exact track count, every title matching, lengths within a couple of seconds, no cover replacement, no MBID conflict. So this is the floor both stand on, not the whole of either test. `Confident` is a rank comparison rather than an equality, so a tier added above "strong" later does not silently stop qualifying.	2026-08-19 02:34:11 -04:00
logan	41c41a860e	feat(explore): carry the local row id on a top result ... `TopResult` was the one projection here that shipped `inLibrary` and no local id, so the top-results cards had no choice but to read the weaker flag. Every sibling model — `MBArtist`, `MBReleaseGroup`, `MBRecording` — already carries `LocalID`, and the candidate builders had the value in hand at every construction site. `LocalID` is set and cleared by a test against `audio_files`, so it means "there is something of mine here". `InLibrary` is written by the same pass but is a one-way ratchet the prune can only clear alongside a local id; it stays for scoring, which is where an approximate answer is fine.	2026-08-19 00:37:57 -04:00
logan	4bf59b45b7	feat(library): answer album completeness for a screenful in one query ... A card grid has to know how much of an album is here — an album held 2 tracks of 10 wearing the same green tick as one held whole is the complaint the badge-accuracy work was filed about — and `GetAlbumCompleteness` is one query per album, which is fifty round trips for a grid of fifty. `GetAlbumsCompleteness` is the same question over a slice. It is two grouping levels rather than the single-album form's correlated subqueries, because a correlated subquery in the FROM clause is not something SQLite will reliably do, and because the slice may only be spelled once or sqlc expands it twice with independently numbered placeholders. An album with no files is absent from the result rather than zeroed: "I have none of this" and "I have no idea" are the third state `Known` exists to keep apart. The test that matters is that the two spellings never disagree — they are genuinely different SQL, so the risk is a drift in meaning (a disc's total counted once per file, a duplicate counted twice) rather than a typo.	2026-08-19 00:37:46 -04:00
logan	4b9114fd8d	fix(tagwriter): declare the track and disc totals when tagging ... An album the user holds 2 of 10 tracks of showed a green tick reading "is in your library", and the mechanism was our own writer. tagwriter wrote track and disc *numbers* and dropped the totals, so autotagging a folder made the release MBID-matched -- which is what earns the tick -- while erasing the one field GetAlbumCompleteness reads. The evidence for "2 of 10" was destroyed by the act that produced the tick. FieldTotalTracks and FieldTotalDiscs are written as the ID3 "n/N" form and as Vorbis TRACKTOTAL/DISCTOTAL; the autotag apply pass and the download importer fill them from the release's own tracklist; and dbsync persists the track total to audio_files.total_tracks so the album page agrees with the file without waiting for a rescan. Five things about it are load-bearing, and four fail silently: - The total is per *disc*, not per release, because that is what the tag form declares and what GetAlbumCompleteness sums per disc. A release total on every file multiplies a two-disc album's expectation by two, which no library can satisfy. backend/tagtotals is that derivation once, since the two callers must not import the writer or each other. - The Vorbis names are TRACKTOTAL and DISCTOTAL and no other spelling. dhowden/tag reads exactly those two keys, so TOTALTRACKS -- which xiph lists and several taggers write -- or a "1/12" packed into TRACKNUMBER writes successfully and reads back as no total at all. The tests therefore assert the round trip through the reader the scan uses, not through the bytes. - ID3's number and total share one frame, so writing either alone must read the other off the existing tag or discard it. A total with no number is not written: "/12" parses as track 0. - The totals are written unconditionally rather than on a diff. The case this exists for is a file declaring no total at all, which compares equal to nothing and is exactly what a "only if it changed" guard skips. - A single-track download is not totalled. A RecordingMBID anchor resolves Expected to that one track, so the same code would tag a track off a twelve-track album "1 of 1" -- and a declared total outranks the catalog total that would have answered correctly. autotag's field constants are a second copy of tagwriter's, deliberately so autotag stays out of the write pipeline's import graph. A key that drifts neither fails to compile nor fails to write -- the writer simply finds nothing under the name it looks for -- so autotagservice, the one package importing both, now pins them. Steps 2 and 3 of the issue stay open under #38: the catalog fallback already landed as completenessAnswer(), and the badge call-site audit is the part that overlaps it. Closes #16	2026-08-18 18:19:54 -04:00
logan	10660c8168	Merge remote-tracking branch 'origin/fix/wanted-without-client' into integration/small-fixes	2026-08-18 11:37:33 -04:00
logan	760021ea5a	fix(downloads): stop searching a list there is nothing to search with ... Every pass attempted every request, each came back "no download clients are enabled", and RecordAttempt wrote that down as an attempt and put a retry on the clock -- so a wanted list built deliberately without a client accrued failures and announced "next check in 6 hours" about a check that cannot happen. Wanting something with no way to fetch it is supported. Being told it is being looked for is a lie, and the row says what is true instead. Everything above the attempt still runs: an artist subscription still expands, and a request satisfied by some other route -- ripped, bought, copied in -- is still retired. Neither needs a provider. TestReconcileRespectsBatchSize now installs a client that finds nothing, because a batch size is about how many requests one pass searches for and that only means something when there is something to search with. Refs #37	2026-08-18 11:26:35 -04:00
yonlu	63ec068add	Merge branch 'main' into fix/small-issue-batch	2026-08-18 15:22:56 +00:00
yonluandClaude Opus 5	185eb1b125	feat(smartplaylist): let a rule set match any rule, not only all of them ... The conditions were joined with " AND " and nothing else, so a smart playlist could only ever narrow: "jazz released after 1960" was expressible and "jazz or blues" was not, which is most of what anyone reaches for a second rule to say. `RuleSet.Match` is "all" or "any", and an empty match is "all" — which is what every playlist saved before the field existed carries, so an upgrade cannot silently widen one. ParseRuleSet rejects anything else rather than falling through to AND, since a playlist quietly returning the wrong tracks is worse than one that refuses to be saved. Under OR each condition is parenthesised and under AND it is not: AND is the tighter operator, so an OR-join has to protect a condition carrying a top-level AND of its own — `days_since_played less_than` is two predicates belonging to one rule. The editor shows the choice as a sentence with the control in the middle, and hides it while there is one rule: with nothing to combine, all and any are the same query. Closes #35 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-18 08:06:16 -04:00
yonluandClaude Opus 5	b3556d825c	fix(mediacontrols): always send an art URL, even when there is no art ... Every other key in the MPRIS metadata map can be omitted safely, because a client reading it renders a track with no title as a track with no title. Art is different: KDE's applet treats an absent mpris:artUrl as no news about the art and keeps drawing whatever the last track had, so playing something without a cover left the previous album's sleeve on screen — which reads as the wrong track playing rather than as missing artwork. The map's construction moves out of UpdateMetadata into a pure metadataMap so it can be asserted on at all: everything else in this file needs a live session bus, which is the same reason the Android contract lives in an untagged file. Closes #41 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-18 08:06:01 -04:00
yonluandClaude Opus 5	bf4f352117	fix(queue): stop claiming a queue came from somewhere it no longer does ... `q.source` was written by SetQueue and cleared in exactly one place, Clear, so no append path touched it: adding a track to a queue built from an album left the page still offering "Playing from <that album>", and since the source is persisted alongside the queue state the wrong label outlived the session that earned it. Every add and insert path drops it now. Removing and reordering deliberately do not — a queue with a track taken out of it is still that album, and the link still goes somewhere true. Only the arrival of a track from elsewhere makes the claim false. The delta event carries the source for the same reason it carries the current index: an append emits nothing else, so the frontend would keep the label it was last given until something forced a full state. Closes #14 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-18 08:05:43 -04:00
yonluandClaude Opus 5	590a0d86dd	perf(library): size the scan to the drive, and prefetch what it reads ... Every parser in `backend/metadata` is header-only -- a few hundred bytes and return -- so on a spinning disk a scan is not waiting on CPU or on bytes, it is waiting on the head to arrive. Two things follow, and the drive says which. **How many reads should be in flight.** This was a flat 2 for anything rotational, which is a pre-NCQ assumption: a modern SATA disk reports a queue depth of 32 and reorders outstanding reads into the order its head passes over them, and was being handed a quarter of what it can use. It gets 4 now. A drive that reports 1 -- a USB bridge, a pre-2004 disk -- services one command at a time in the order given, where every extra worker is one more seek competing for one head and the scan gets *slower* the harder it is pushed; that keeps 2. **And that the next seek should already be queued.** A prefetch stage between the walk and the workers issues `POSIX_FADV_WILLNEED` over the first 512 KB of each file -- enough for an ID3v2 tag carrying cover art, or FLAC's STREAMINFO and PICTURE blocks. The buffered channel *is* the lookahead: the goroutine runs 16 files ahead of the workers, hinting as it goes, so the read a worker needs has been in flight for sixteen files' worth of parsing by the time it asks. Rotational only; an SSD gets the channel back unwrapped and pays nothing, since it has no seek to hide and already has one worker per core. `workersForProfile` is the policy on its own so it can be tested against drives this machine does not have, and the scan logs the device, its rotational flag and its queue depth, so the decision is inspectable rather than inferred. Also: `ScanConcurrency` has been a validated three-value config field with exactly one caller, passing the constant `auto` -- so choosing `ssd` or `hdd` by hand did nothing at all. It reads the config now. The two modes overrule detection about the *disk* and not about its queue, since a user who picks `hdd` on a queueing drive still wants that drive's queue used. What is not here is inode-ordered dispatch. It needs the streaming walk restructured to buffer per directory, and with queueing the drive is already reordering what the hints put in front of it; that wants a measurement on real hardware before the complexity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MeQt5hgXg5YGoNZQ9ozG7L	2026-08-17 22:12:15 -04:00
yonluandClaude Opus 5	36af7090d9	fix(system): resolve a path to its own disk, not the first on its major ... `deviceForPath` scanned `/sys/block` comparing device numbers and, when no entry matched exactly, took the first one whose *major* agreed. Every SATA disk is major 8. A filesystem's `st_dev` is its **partition**, so the exact match never hits for anything on one, and the fallback then resolved `/dev/sdb3` to whatever `/sys/block` listed first -- which is alphabetical, which is `sda`. On the machine this was found on that is a Samsung SSD sitting next to the 6 TB spinning disk the library is actually on, so `IsRotationalDisk` answered false and the scanner ran one worker per core across a drive with one head. Matching on major alone cannot be right on any machine with two disks, which is the case this exists for. It goes through `/sys/dev/block/<major>:<minor>` instead -- a symlink the kernel maintains to the device's own sysfs directory -- and climbs to the parent when that turns out to be a partition. One readlink, no scan, no ambiguity. The dev_t decode goes with it: Linux packs 12 bits of major and 20 of minor split across the word, and masking the low byte of each is right only for the first 256 of either. `ProfileForPath` returns what the scanner needs to ask next, and the new half is `queue_depth`: how many commands the drive will accept and reorder at once. A SATA disk with NCQ enabled reports 31 or 32 and one without reports 1, which is the difference between concurrency helping and hurting. An absent file is read as "queues", because everything that does not publish it -- NVMe, virtio, device-mapper -- is a device where concurrency is fine. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MeQt5hgXg5YGoNZQ9ozG7L	2026-08-17 22:11:51 -04:00
yonluandClaude Opus 5	3e142f8c35	test(downloads): guard the service fixture on something the fake sets ... `newServiceFixture` stops auto-pick from starting a grab, because none of its tests is about the download and a detached `go m.grab(...)` racing `t.TempDir()`'s cleanup is how they fail. It did that with `MaxSizeMB: 1` -- and the size gates read `Candidate.TotalSize`, which real providers fill and the fake leaves at zero. Zero is under every ceiling, so the guard never fired and the race it was written to prevent kept happening, roughly one run in fifteen: TempDir RemoveAll cleanup: unlinkat ... : directory not empty The guard is a format the fake never produces. Thirty consecutive whole-package runs, none. `TestManualDownloadSatisfiesRequestOnSuccess` was relying on the guard being broken -- it is the one test here that wants the download -- so it now clears the preferences itself rather than depending on a bug. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MeQt5hgXg5YGoNZQ9ozG7L	2026-08-17 22:11:10 -04:00
yonluandClaude Opus 5	3d375adab1	feat(downloads): bound auto-pick by bitrate, and take a good copy ... Three faults, one subsystem, and the middle one is why a request that looked obviously satisfiable came back refused. **The guardrails were in megabytes, which cannot mean anything.** 300 MB is a generous FLAC single and a suspiciously small boxset, and whoever fills the field in has no idea which release the pipeline will apply it to. `MinKbps`/`MaxKbps`/`PreferredKbps` are the same statement divided by how long the music is, so one number holds across a nine-minute EP and a three-hour opera. The runtime comes from `Download.Expected`, which every anchored request already carries, so this costs no lookup; the rate is audio bytes over that, falling back to the mean stated per-file bitrate when the runtime is unknown. Artwork is excluded from the numerator, or a folder with 30 MB of scans reads as a better rip. An unknown runtime *passes* the window rather than failing it: the window is a statement about quality, and refusing everything the moment MusicBrainz is missing a track length would be a silent embargo. `MaxFileSizeMB` survives as a separate ceiling, still in megabytes on purpose -- it is a question about disk space, and it has to apply to a candidate whose bitrate cannot be worked out at all. **Auto-pick required daylight over the runner-up**, 0.08 on the combined score, and so fired hardest in the case it was never written for: a popular album turns up five *correct* copies, all matching the tracklist at 95%+ and differing only in format and seeders, their scores land within a point of each other, and it refused forever on the grounds that the choice was the user's. It was not. There was no question about what to fetch, only about which copy -- and abundance is the condition under which that matters least. A candidate no longer has to beat the field, only clear the bars on its own terms; where several do, ranking puts the one closest to the preferred bitrate first. That tie-break needed the preference to carry weight or it would have been decorative in a new unit: `BitrateFit` was 0.05 against format's 0.42, so asking for 320 and being handed a FLAC every time was the designed behaviour. When a preference is set the weights shift to fit 0.40 / format 0.20 / bitrate 0.10, taking it off the two heuristics that exist as stand-ins for the preference the user has now given. Health and priority are untouched. And the fit spans 0.5 to 1.0 rather than 0 to 1, so a preference can promote the copy that matches it and can never push the others under `minQuality` -- turning "I like 320" into "never take anything else" silently is what `MinKbps`/`MaxKbps` are for, out loud. **And a refusal quoted numbers that passed.** The request list built its message from `ranked[0]` -- the best candidate *before* the guardrails and before the lead check -- so a request killed by the size window, or by having too many good copies, reported "best of 12 found is not a confident enough match (match 96%, quality 88%)". `AutoPickVeto` names the gate that actually refused, and `AutoPickable` is that returning empty. Existing configs: the old `MinFileSizeMB`/`PreferredFileSizeMB` are not migrated. A number meaning "300 MB" cannot be reinterpreted as a rate without knowing the album it was aimed at, so carrying it over would be inventing an intent nobody expressed. Those two fall back to no window, which is the permissive default and what a fresh install gets; `MaxFileSizeMB` carries over unchanged, because a ceiling on bytes still means exactly what it did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MeQt5hgXg5YGoNZQ9ozG7L	2026-08-17 22:10:51 -04:00
yonluandClaude Opus 5	40984f6086	fix(explore): let a slow archive node finish, and read the 404 back ... Explore's album art was almost entirely missing: 5 of 24 cards on the shelves had a cover, and those five were the ones already on disk. The Cover Art Archive answers `front-250` with a 307 to an Internet Archive storage node, and those nodes are slow. Measured against the twelve albums on Explore's own shelves, a successful fetch took 14-16 s and a failing one 13-17 s, against a client timeout of 10. So every live fetch died, and a timeout writes nothing and says nothing -- which is why this reads as "Explore has no album art" rather than as a slow upstream. The timeout is 30 s, chosen to clear the measured range: the fetch is off the critical path, so waiting costs nothing and giving up early costs the whole page. Two things beside it, both found on the way. `writeCache(mbid, nil)` has recorded "the archive has no art for this" as an empty file since it was written, and nothing has ever read it back: `readCache` returns "" for an empty file, which is indistinguishable from a miss. So every art-less release group was re-fetched from CAA on every render that asked about it. A third of the shelves are art-less, so that was a third of the page spending a live request to be told again what the last one said. `knownMissing` reads it, on both the release-group and the release path. And the frontend marked a failed fetch as permanently answered for the session, so a timed-out cover never retried within it. It drops the marker instead; a genuine 404 is now answered from disk, so re-asking one costs nothing. Measured after: 23 of 24. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MeQt5hgXg5YGoNZQ9ozG7L	2026-08-17 22:09:14 -04:00
logan	1940cb548f	fix(test): stop asserting a cache hit against a one-second deadline ... TestCacheTTLExpiry set a 1s TTL and immediately asserted a hit, so it depended on an upper bound of elapsed wall-clock time between Set and Get. Nothing can promise that: on the capacity-1 runner, with the rest of the suite running in parallel, the goroutine can be descheduled for longer than the TTL and the entry is then correctly gone. It failed that way on this PR while passing five times out of five locally, and it touches no code this branch changed. Two entries now: one with an hour to live carries the presence assertions, one with a second carries the expiry. Sleeping past a TTL is always safe, so only the direction that cannot flake is timed.	2026-08-17 19:55:22 -04:00
logan	3d65da0529	test(download): stop racing a download these tests never wanted ... `check` failed on main with two failures in one package, and they are one cause wearing two shapes: service_test.go:66: state = "satisfied", want wanted testing.go:1369: TempDir RemoveAll cleanup: ... directory not empty Every test in service_test.go is about the durable Request that StartDownload leaves behind, and none is about the download. But the fixture is an anchored four-track request with a healthy provider, which is precisely what AutoPickable says yes to -- so Manager.Start fired `go m.grab(...)`, detached and with context.WithoutCancel, and the tests raced it. Measured: the request reaches "satisfied" about 100ms after StartDownload returns, so the first failure is the assertion reading the next state, and the second is that same goroutine still writing into t.TempDir() after the test returned. The fixture now puts the candidate outside the auto-pick size window, so the grab never starts. That is better than waiting for it: with no goroutine there is nothing to be slow, and the tests state what they mean without a timing assumption underneath. A test that does want the download uses managerFixture and sets its own preferences. It passed 20 runs under CPU load, but so did the broken version -- this is a CI-only failure locally, so the cause was proved directly instead: with the fixture's old preferences the request is observably "satisfied" within 100ms of StartDownload, which is what CI read.	2026-08-17 14:16:45 -04:00
yonluandClaude Opus 5	4f8257ef72	fix(database): never retire the catalog the index build derives ... The stale-shape repair dropped the CI catalog on its first run: retiring a table ... table=explore_index reason="column entity_type is TEXT, schema declares INTEGER" index maintenance mode=build reason="no completed import yet" The mismatch was real and the drop was correct by the app's rule: a client's catalog is *downloaded*, so a wrong shape costs a minute of re-fetching the artifact, while keeping it costs every Explore read. It is the wrong rule for one database. cmd/indexbuild's catalog is not downloaded, it is what the artifact is cut from — the only way back is the ~205 GB dump stream the /cache volume exists to avoid. And that database is deliberately kept in the older encoding, which `fix(indexexport): read an index older than the binary` exists to tolerate, so the shape does not match by design and would have been dropped on every run. retireLibraryTables, right beside it, never touches the catalog for exactly this reason. The repair reached past that protection because it runs inside database.NewDB, which cmd/indexbuild also calls. So the policy is a build tag, which is how this project already tells the index tools apart (runtime_indexbuild.go, servicestartup.go, dumpbuild_stub.go): Cache tables are rebuilt in the app and never in cmd/indexbuild. Owned and Derived are still repaired in both — that is the half this database can safely discard, and retireLibraryTables already discards it. The residual trade is deliberate: a future explore_index column will now fail the index job loudly on applySchema rather than silently costing it a 205 GB rebuild. A human should decide that one. TestTheCatalogSurvivesAStaleShape is the accident, symptom first, with the shape the real database is in — every current column, ids and entity type still text. It fails with "the catalog was retired" when the policy is flipped back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AfVYUVExXsx1nSWrXN8mAh	2026-08-17 12:24:49 -04:00
yonlu	b505959934	Merge remote-tracking branch 'origin/main' into wails-v3	2026-08-17 11:38:49 -04:00
logan	de2b324e20	feat(explore): refuse 0.6 GB on someone's mobile data ... Plan 016 B4. The catalog artifact is about 0.6 GB and the app fetched it with no awareness of the connection: on a desktop that is a minute of bandwidth, on a phone it can be a month's allowance. It is now skipped on a cellular connection unless `AllowMeteredCatalogDownload` is on, with the toggle in Settings' Search Index section, where the text explaining what the catalog is already lives. The file layout is dictated by the cgo rule rather than by taste. `explore` is imported by `cmd/indexbuild`, which builds with CGO_ENABLED=0 and must not link Wails, so `netpolicy.go` holds the policy and the JSON parsing -- tested on every platform -- and the single platform call is a closure injected from `app.go`, which already names `application` legitimately. Three rules in it are load-bearing. An unknown answer is not a metered one: only mobile answers at all, and treating silence as metered would have disabled the download for every desktop user in the world. Cellular is the only signal available, because the runtime reports `wifi|cellular|ethernet|none` and no metered flag -- so a metered Wi-Fi cannot be detected and is not refused, which is documented rather than implied. And the gate runs before the first status write, so declining is a no-op instead of a job in the indicator and an error tier to dismiss. Two corrections to the plan while implementing it: the portable API is `application.Mobile.NetworkJSON()`, not `application.Android`'s, which exists only under the `android` build tag; and the permission is read at the moment a download would start, so enabling it takes effect on the next attempt rather than the next launch.	2026-08-17 10:48:00 -04:00
yonluandClaude Opus 5	409bfd5e89	test(download): wait for the work, not for the state that precedes it ... TestManagerEndToEndAutoPick waits for StateComplete and then asserts that staging was released and the library was rescanned. Those happen *after* the state is recorded: manager.go sets StateComplete, then satisfies the request, then releases staging, then scans. So waiting on the state is not waiting on either assertion, and on a loaded machine the worker is descheduled in between and the test reads the world one step too early: manager_test.go:209: staging not released: 1 dirs remain manager_test.go:218: library scans = 0, want 1 It passed alone every time and failed three times under a full-suite run, which is the signature of a test race rather than a broken manager — nothing here is wrong except what the test chose to wait on. It blocks pushes, since the pre-push hook is exactly the loaded run. It polls for the side effects now, through the waitFor this package already has and already uses for the same reason one file over (service_test.go waits for a request to become satisfied after the same StateComplete). Not reproduced on demand: eight spinners and -count=5 did not provoke it with or without the fix, so this rests on the ordering being plain in the code rather than on a red-to-green demonstration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AfVYUVExXsx1nSWrXN8mAh	2026-08-17 09:22:51 -04:00
yonlu	4fc0cdeab7	Merge remote-tracking branch 'origin/main' into wails-v3	2026-08-17 08:29:21 -04:00
yonluandClaude Opus 5	eb059a3d71	fix(database): retire a table whose shape the schema moved past ... `applySchema` is CREATE ... IF NOT EXISTS and there is no migration chain, so a *changed* table never migrates: the statement silently no-ops against the old shape. Two plans had already landed on that, and neither showed up in a test because a fresh install is perfectly healthy. - 014 added `total_tracks` to explore_index and to `indexRowFields`, the projection every explore read uses, so every search, browse, artist page and album page failed with "no such column: total_tracks" on any database that already had a catalog. - 013 reshaped audio_files, so applySchema could not run at all and the app did not open. staleshape.go runs before applySchema and drops what disagrees, so the create is a create. It parses sql/schemas/ for the expectation rather than writing the column list down a second time, and it notices a changed *type* as well as a missing column — 013 moved mbid TEXT to BLOB, which no ALTER could express and which SQLite will not coerce, so a query against 16 raw bytes returns no rows rather than an error. Only Authored tables are exempt. Cache is rebuildable by definition, Owned is what a rescan rebuilds (plan 013's stated "delete and rescan"), and a table the schema no longer describes at all goes too -- 013 left seven behind plus schema_migrations. Three things in it are load-bearing, and each was a bug first: - The parser read `UNIQUE(mbid)` as a column, which made a healthy catalog look stale. That would have retired it on every launch and cost every user an artifact download per start. - The drops are one transaction with defer_foreign_keys. Those legacy tables reference each other, so any order fails on whichever goes first; turning foreign keys off instead would suppress playlist_tracks.audio_file_id's ON DELETE SET NULL and leave entries pointing at ids a rescan reissues to *different songs*. Nulled entries are empty; stale ones are wrong, and wrong quietly. - The order is sorted, so a failure reproduces. Map order is random, and the foreign-key bug passed its own regression test on two runs in three until the order was fixed. Verified against a real pre-013 install: it opens, its 22 playlists survive, 1,887 linked playlist entries become 0 rather than dangling, and the legacy tables are swept. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AfVYUVExXsx1nSWrXN8mAh	2026-08-17 08:27:05 -04:00
yonluandClaude Opus 5	b3737d30af	feat(explore): carry multi-artist credits in the catalog ... A track credited to more than one artist has exactly one navigable artist in this app and the rest are punctuation. `primaryArtist()` string-parses the credit, strips a " feat. " clause and discards the guest; it deliberately does not split on "&", "with" or "," because those live inside real artist names. Measured on a real 26,069-file library plus an 80+80 MusicBrainz sample: 13% of recordings are multi-artist upstream, while only 0.86% of files carry any structured multi-artist tag — mp3 carries zero files with multiple MUSICBRAINZ_ARTISTID across 19,840. Of 1,286 files saying "feat.", 90% have nothing structured behind it, and a sample of 80 such files was multi-artist in MB 80 times out of 80. CLAUDE.md justified plan 013's removal of the credit tables with "3 credits of 2,823 listed more than one artist". That measured our own *writer* — cachedLinkArtist was called once per credit, so a collaboration could never have been recorded. Dropping the join table was still right on cost; the evidence for "multi-artist is rare" was not. A credit is ordered parts and the credit string is derived from them, so join phrases are assembly instructions, not disassembly ones. Nothing here reconstructs a credit by searching a name inside a credit string: the stored text may come from tags while the parts come from the catalog, and those disagree for ~1 in 3 multi-artist credits. Where it comes from, after two dead ends: the canonical dump CI already streams has no join phrases and no as-credited names, and the JSON dumps cover 153,691 recordings of ~35M with *zero* overlap against a real library. So mbdump.tar.bz2 — 7.1 GB, ~13.7 min in pure-Go bzip2, whose members are alphabetical, which is what lets one pass resolve an entity's credit without buffering 35M recordings. - artist_credit_part / artist_credit_ref, multi-artist credits only: a single-artist credit is already explore_index's own artist_name. - Column layouts verified against the real 20260815 export; ErrDumpShape makes a wrong guess a failed build, not a wrong catalog. - The pass runs on every mode, not just a build. The job picks its mode from the index's own state, and a complete import means "refresh", which never enters the importer — so credits could otherwise only arrive via a rebuild that re-downloads ~205 GB. It reports whether it populated anything, which is what flips `changed` and republishes. - The importer asks whether an artifact carries the tables, on the writer where `core` is attached, so the artifact already published still imports. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AfVYUVExXsx1nSWrXN8mAh	2026-08-17 08:25:36 -04:00
logan	da38b865fc	feat(android): playback that survives the screen locking ... An app that plays audio becomes a music player at the point where the screen can lock, a call can interrupt, and the headphones can come out. None of that existed: the foreground service was typed for media but had no MediaSession, no transport notification and no audio focus, so oto would happily keep writing to a stream nobody could hear. The apparent blocker is that Wails' androidBridge* helpers are unexported, so Go cannot call arbitrary Java. It does not need to. StartForegroundService(json) *is* exported, and build/android/ is our tree, so widening the JSON WailsBridge already accepts is a local edit; coming back, WailsBridge.emitEvent lands on the application event bus, which Go subscribes to with app.Event.On. One document out, one command event back, and no new JNI. No new Gradle dependency either: minSdk is 21, which is exactly when android.media.session.MediaSession and Notification.MediaStyle arrived, so androidx.media buys two Build.VERSION branches' worth of nothing. Four things in it are load-bearing. **A duck is not a volume change.** Player.SetDuck holds the attenuation as an offset and re-applies the user's level through setVolumeLocked, so it cannot accumulate across repeated ducks and getUserVolume -- which feeds the event, the persisted state and every relative change -- still reports what the user chose. Writing through to the volume would let one notification tone permanently turn the music down. **The duck path is pre-Oreo only.** From API 26 the framework ducks the app itself and sends no CAN_DUCK focus change; asking to be told instead (setWillPauseWhenDucked) would mean pausing for every notification tone, and doing both would attenuate twice. **An unchanged payload is not an event**, the rule emitStatus already states one package over: every push crosses JNI and re-delivers an Intent, and the player pushes state on several paths that can agree. **After the first start, an update is startService.** From Android 12 a background app may not *start* a foreground service but may keep feeding one it already has, which is every track change with the screen off. Relatedly, every path through onStartCommand calls startForeground -- one that returns without it is killed. The contract with Java lives in androidpayload.go *without* the android build tag, and is tested. Everything left in android.go is untested by construction: make lint and make test are three tag sets on linux/amd64, so the only thing that compiles it is the cross-compiler in make android, and the only thing that can run it is a phone. None of the behaviour above has been observed on a device. The APK builds and both halves compile; that is the whole of what is verified.	2026-08-16 22:26:03 -04:00
logan	e14a34fccf	fix(android): let the app reach the user's music ... Three of plan 016's four blockers. Each is a different reason the app could not work at all on a phone. **It had no permission to read anything.** The generated manifest asked for INTERNET, VIBRATE, biometrics, location and a camera, and nothing whatever about storage -- so at targetSdk 35 the app could see its own private directory and no music. It now declares READ_MEDIA_AUDIO, the two capped legacy storage permissions, and MANAGE_EXTERNAL_STORAGE. That last one is deliberate and is the load-bearing choice. This app is a library manager: audio_files.file_path is the primary key of ownership, the scanner walks a directory the user chose, and tagwriter rewrites files in place. MediaStore offers no stable directory to walk and no in-place write, so scoped storage is not "more work" here, it is a different application. MANAGE_EXTERNAL_STORAGE is Play-restricted, which is acceptable only because this ships as an APK through the package registry -- if it ever targets Play, that line is what has to go, and plan 016 says what replaces it. It is granted on a Settings screen rather than in a dialog, so it cannot be requested with requestPermissions(). MainActivity opens that screen on every cold start until access exists -- there is no degraded mode worth offering -- and re-checks in onResume, because the way back from another task is a resume, emitting android:storageAccess so the frontend can react. **The first-run flow could not complete.** All three call sites asked for a folder through the Wails dialog, which returns an error on Android: SAF yields tree URIs and this app is keyed on paths. So the app browses the filesystem itself, which it can now do. ListDirectories lists directories only (the thing being chosen is a library root), skips what it cannot stat rather than failing the listing (Android's storage root holds directories no app may enter), follows symlinks (os.DirEntry reports the link, so a symlinked music folder would silently vanish), and hides dotted entries. utils/pick-directory.ts is the one place that chooses between the two, so the three call sites changed by one line each. **Which platform is asked of the backend**, not of System.IsAndroid(): the dialog is backend code, so the backend is what knows whether it can open one; it answers for iOS at the same time; and it keeps the fallback testable through the ordinary transport fake rather than a module mock of the Wails runtime, whose platform helpers read build constants. **And MPRIS was compiled into the Android build**, because android implies the linux build tag, so it went looking for a session bus that does not exist. mpris_linux.go is `linux && !android` now and the stub covers Android, which means no lock-screen transport there yet -- a missing feature rather than a broken one, and the remaining blocker. The foreground service is typed mediaPlayback rather than the scaffold's dataSync, with the matching permission, so playback can survive the screen locking once there is a MediaSession to drive it. The type in the manifest and the one passed to startForeground must agree or startForeground throws.	2026-08-16 17:18:03 -04:00
logan	0c7f34ab90	fix(android): give the app a home directory so it starts ... backend/system resolves config and data from $HOME or the OS equivalent, and Android has neither: buildUserDirPath switches on runtime.GOOS with cases for darwin, linux and windows and a default returning errUnsupportedOS. So NewYellowJacketApp failed and main() called os.Exit(1) about six milliseconds after the JNI bridge came up. That failure is invisible in all three places anyone would look. There is no panic, no AndroidRuntime stack and no tombstone, because os.Exit is not a crash; Go's stdout does not reach logcat, so the slog line naming the error is discarded; and ActivityManager respawns the process fast enough that pidof always answers, so a crash-looping app looks alive. main() now sets the override before anything asks for a path. application.Mobile.StoragePath() is the platform's own answer -- getFilesDir() on Android, Application Support on iOS -- and returns "" on desktop, where UseHomeOverride is a no-op, so this needs no build tag and changes nothing off mobile. resolveUserDirPath already honours YJ_HOME on every OS, so there was a seam for it. The knowledge stays in main(): backend/system gains no import of the Wails application package, for the same reason backend/events is split by the indexbuild tag. UseHomeOverride's two rules are tested because nothing else would notice them breaking. An empty base does nothing, which is exactly the desktop case. And an override already set wins, so YJ_HOME still relocates a sandbox on the one platform that would otherwise decide for itself. This is not the end of the port. The app now reaches the database and takes SIGSYS on the x86_64 emulator -- modernc.org/libc issues a raw lstat syscall on linux/amd64 and Android's seccomp forbids it. arm64, which is what ships to phones, has no lstat syscall at all and routes through fstatat, so it is structurally unaffected. See NOTES.md.	2026-08-16 16:25:20 -04:00
logan	b98840ee37	fix(build): keep the index tools free of the Wails application ... The v3 migration put application.Get() in backend/events and a ServiceStartup hook in backend/explore, both of which cmd/indexbuild reaches. v3's application package is GTK/WebKit bindings on Linux, so the index-artifact job — a plain golang container with CGO_ENABLED=0, on the stated grounds that neither command imports the app — stopped compiling with "undefined: pointer". That job owns the ~205 GB dump checkpoint, so it is the worst place to learn this. Both are behind the indexbuild tag now: the one app.Event.Emit lives in runtime_wails.go, runtime_indexbuild.go answers ErrNoRuntime (what the app itself returns before Run, so Deliver's callers need no second path), and explore's ServiceStartup moves to its own tagged file. TestIndexToolsDoNotImportWails walks `go list -deps -tags indexbuild` so the claim the workflow makes is checked rather than assumed.	2026-08-16 14:51:01 -04:00
yonluandClaude Opus 5	e7748f1fd5	feat(database): shape the library like files, and shrink the catalog ... Plans 013 and 014, the album page that prompted them, and the smaller fixes they turned up. Changelog, largest first. ## The local library is shaped like files, not like MusicBrainz `audio_files` carries its own tags and points at `albums` and `artists`; `file_genres` is the one real many-to-many. `recordings`, `release_group_recordings`, `artist_credit`, `artist_credit_artist`, `recording_genres`, `release_groups` and `release_to_rg` are gone from the local side, and with them a six-way join in every read, a `MIN(release_group_id)` subquery in eleven queries and a first-credited-artist subquery in nine. Measured on a real 25,966-file library, every many-to-many that model expressed was 1:1 in the data. - Ownership is a file. `GetFilePathsByRecordingMBIDs`, `LibraryMBIDIndex.CheckMBIDs`, `collectLibraryEntities` and `pruneStaleLocalCrossReferences` all join `audio_files`, so the 812 orphaned recordings, 216 release groups and 260 artists that library carried are now structurally impossible. - One projection: every track query selects from the `track_metadata` view, one row type, one mapper. Nine hand-rolled copies had drifted far enough to report different years on different screens. - `library_id = 0` means every library, so each list query exists once instead of scoped and unscoped with a branch at every call site. - No migration chain. `sql/schemas/` is the one description of the shape; `sql/migrations/`, `applyMigrations` and `schema_migrations` are squashed away, along with the drift between them that had sqlc generating against a stale schema. - `database.InsertTestTrack` is the one test seeder; twenty test files had been assembling the old FK chain each in its own order. ## The catalog stores its ids as bytes `explore_index`'s three 36-char MBID columns and its entity-type text are 16 raw bytes and a small integer. The table and its six indexes go 780 MB to 405 MB on a real 2,052,200-row catalog, which is why a fresh install is ~0.6 GB rather than ~1.0 GB. - `backend/explore/mbid.go` is the only place the encoding is known; everything above it speaks dashed strings. - `CHECK(length(mbid) = 16)` makes a stringly write fail at the insert rather than silently returning no rows, since SQLite does not coerce between TEXT and BLOB. - The importer asks the artifact what encoding it carries and converts on the way in, so the artifact already published keeps working and no format bump is needed. - `indexRowColumns`/`scanIndexRow` replace four copies of a 22-column list, and `TestStoredEncodingRoundTrips` sweeps every read path. ## An album page that says how much of the album is yours - One question, asked once: is there a file. `filePaths` is filled by a single batched lookup when the tracklist settles, and the badge, the Play count, the dimmed rows and every menu item read it — replacing four claims of decreasing confidence that could show a green tick on an album whose every action did nothing. - Play, Play 7 of 12, or no play button at all. - `total_tracks` on `explore_index` (~2 bytes over 400,677 release groups) and on `audio_files` from tags that have always carried it: a complete MBID-matched album now makes no catalog call at all, where it used to spend the most expensive request the app makes. - A merged cluster shows the running order the most releases agree on, and the version list marks the release you own rather than standing a synthetic entry in for it. - `AlbumReleasesFailed`: a slow fetch is no longer reported as a failed one by a 12-second timer. - Rows not in the library are dimmed in place (with `aria-disabled`) instead of the owned ones wearing a green tick and a legend. ## Caches and cover art get ceilings - Only the three tiers of a cover are stored; the full-resolution copy nothing rendered was 1,134 MB of a 1.4 GB covers directory. - One artist portrait is downloaded and the rest are remembered as URLs — 4.1 GB of a 5.3 GB cache was candidates no code path reads. - `browsedArtBudget` and `httpCacheBudget` bound what an age cannot: the same install held art for 5,770 artists in a 1,301-artist library. - `OrphanedArtistImagesJob` joined a bare MBID onto a sharded directory, so it deleted the rows that were the only record of the files it left behind. `explore.ArtistImageDir` is that layout's one definition now. ## The autotag queue asks whether there is work `tagging_items` was a row per album folder, not a queue, and no query read the `tag_status` column that held the answer. The four queue queries ask the files, which matters most where it is least visible: `startPrefetch` was scoring every album in a tagged library against MusicBrainz. ## Phantom playlist tracks resolve in place An M3U8 imported before its files leaves phantom rows; they now match by path and fall back to position, keep their place in the playlist when resolved, and pair best-first so two phantoms cannot claim the same file. ## Playing a track plays the list it is in Double-click, and Play on a single row's menu, queue the list as displayed with `startIndex` on that row — the album page and the track list used to queue one track and discard the album around it. A multi-row selection still plays exactly itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AfVYUVExXsx1nSWrXN8mAh	2026-08-16 13:58:15 -04:00
yonluandClaude Opus 5	deb3f3da7e	feat(wails): move the e2e harness and headless launch onto v3 ... make e2e is green on chromium: 92 passed. The harness is rebuilt on what v3 actually offers, and three of the four things it replaced turn out to be better than what they replaced. The headless launch is v3's own server mode. scripts/dev-headless.sh ran a `-tags dev` binary whose app_dev.go parsed -devserver/-assetdir out of os.Args; that file went with v2, so the harness had no server at all. `-tags dev,server` is a first-class mode and needs no display, so Xvfb is gone from the script and from CI. The bridge hooks two places, neither of them EventsOn. Inbound is window._wails.dispatchWailsEvent, wrapped by pre-creating the object the runtime keeps and putting an accessor on the one property. Outbound is fetch: v3 routes every runtime call through one POST, so the bridge sees binding calls and event emits from any module, needs no walk of an object graph, and cannot miss a call made before it looked. __yjEvents.call posts to that endpoint by method name, so it depends on nothing in the app's bundle and works on a page with no init script. That is what lets seed-sandbox.sh drop playwright-cli entirely — it drove AddLibrary through a browser only because window.go was v2's one way in — and with it a global npm install and a second Chromium in CI. measure.mjs and one spec lose their window.go walks and read the bridge's log instead; e2e/support/method-ids.mjs derives id -> name from frontend/bindings/ (phase 6b option 1, so it cannot go stale silently). Plain .mjs because measure.mjs runs under bare node and one derivation beats two that can disagree. Four bugs surfaced, and the migration is how. The cross-service wiring never ran headless. It hung off Common.ApplicationStarted, which server mode never emits — setupCommonEvents is an explicit no-op there — so the queue had no TrackLoader and playing a track changed the queue and then silently did nothing. It is a service registered last now (backend/startup.go): services start in registration order, which is the ordering the wiring needs, in every mode. Six specs called SetQueue with 3 of its 4 arguments. v2 accepted that and filled the gap; v3 answers "expects 4 arguments, got 3". requested-badge's cleanup read window.go and returned early on `if (!svc)` — the silent cleanup its own comment was written to prevent, one migration later. It posts to the runtime endpoint now, which any page can do. SearchIndex.Search trusted a startup latch, so rows a spec staged afterwards were unsearchable and three specs passed only when an earlier one happened to flip it. shelves.go fixed exactly this and left hasCatalogRows behind; the search path now uses it as the fallback, with the latch still the fast path. Two spec edits are deletions of assertions about v2. harness.spec checked Object.keys(window.go) and that a bad call *hung*; it now checks the real runtime is loaded and that the backend rejects with a TypeError naming the argument. album-actions asserted a tracklist legend that dcc40b1 deleted on main — that spec has been failing since, and what replaced it is covered in frontend/test/components. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UDCbcCZQepnpSQYJ6SxxZm	2026-08-14 20:58:20 -04:00
yonluandClaude Opus 5	4471db3aef	feat(wails): move the Go side to v3 ... Phases 2 and 3 of plan 009, plus the parts of phase 1 that could not land before them. Nothing in the tree imports wails/v2 any more; all three lint and test configurations are green and `go build .` produces a running binary. The point of the migration is one file. backend/events/emit.go probed ctx.Value("events") — a v2-*private* context key — to decide whether emitting was safe, because runtime.EventsEmit called log.Fatalf on a context without the runtime and took the process down with it. v3's emit takes no context, so that is now application.Get() == nil. D1 held: events.Emit keeps its ctx as the WithSink test seam, and all 45 call sites and 7 test files are untouched. The bootstrap splits into application.New + Window.NewWithOptions + Run. Ten bound services implement ServiceStartup instead of being handed a context by hand from OnStartup, which also stops ten SetContext methods being exported as bindings. jobs.Registry and explore.SearchIndex keep theirs — neither is bound, so converting them would be churn for no binding removed. Four things differed from the plan and are written up in it: GPU policy moved to the per-window LinuxWindow options rather than surviving on LinuxOptions; there is no OnStartup/OnDomReady option, so app-level wiring hangs off ApplicationStarted; application.NewService is generic, so FEBindings []any could not survive (the binding generator is a static analyser and would have seen nothing); and the quit veto had to be restructured, because v3's dialog answers on a callback rather than returning the button, so ShouldQuit vetoes, asks, and quits again from the callback. Window state saving moves to a WindowClosing hook — the size has to be read while the window still exists, and v3's OnShutdown has neither context nor window. backend/logging is deleted rather than ported: v3 takes a *slog.Logger directly, so the v2 logger.Logger adapter had no caller left. Phase 1's tail rides along, now that it can: the Makefile's wails invocations, all 50 webkit2_41 sites, lefthook, both packaging recipes and ci.yml's apt lists. v3 builds against GTK4 + WebKitGTK 6.0, which Arch and ubuntu:24.04 both ship, so the tag is a deletion rather than a translation. Phase 4 is next and the branch is not usable until it lands: the app builds, but frontend/wailsjs/ is v2's tree and nothing regenerates it, so the frontend cannot reach the backend yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UDCbcCZQepnpSQYJ6SxxZm	2026-08-14 14:01:02 -04:00
yonluandClaude Opus 5	20fbf28f2a	perf(explore): make the owned-artist backfill yield, mark, and stop ... The post-scan backfills share MusicBrainz's rate limiters with every page the user can open, and both were FIFO — so a thousand-artist enrichment put an album page behind an hour of queued work. WithBackgroundLane/WithBackgroundPriority add a slower second lane: a marked wait takes no token while any interactive wait is outstanding. It is a context marker rather than a parameter because a backfill calls the same client methods a detail page does. A long backfill also has to be visible and stoppable, so jobs.KindCatalogEnrich registers both with progress and cancel — after the work is counted, since these passes are a no-op on every launch once the library is covered. What it does not fetch is the point. It ran for hours against a 900-artist library and marked nothing, because three of the four things it did per artist were work nobody asked for: similar artists, which the artist page already resolves on view, and a full GetArtistImage (fanart.tv, TheAudioDB, Wikidata, Wikipedia, ten portraits) reached only to warm the MB artist lookup EnsureArtistRels does alone. It was also serial across artists while every limiter is per-host and idle. The marks are a table rather than more explore_index columns, because artifactimport merges by column list and a flag added there is a second place to remember. BrowseReleaseGroupsAll pages to exhaustion, where the old call silently cut a prolific artist at 100 release groups. One portrait is downloaded now; the rest are remembered as URLs. resolveAllSources downloaded every candidate, up to ten, full size, while nothing reads anything but primary.jpg — 5.3 GB measured on a real cache, 4.1 GB of it unreachable. OrphanedArtistImagesJob is why that survived: it joined the bare MBID onto the images directory, but artist directories are sharded under a two-character prefix, so it named a path that never existed and deleted the rows that were the only record of the files it left behind. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UDCbcCZQepnpSQYJ6SxxZm	2026-08-14 13:33:54 -04:00
yonluandClaude Opus 5	878cf4b561	fix(playback): submit a durability write, do not perform it ... Every write goes through one connection — MaxOpenConns(1), because SQLite has one writer — and a background pass can hold it for a long time. The player and the queue wrote inline from paths that hold their own mutexes, so a contended writer did not merely slow persistence down: SetQueue blocked in LoadFile's saveState and then in persistState, while holding q.mu and p.mu. That is the exact shape of the report: the track changed and the transport sat at paused, nothing appeared in the queue, and the play button did nothing because Queue.Play waited on the same held q.mu. Diagnosed by profiling the running app — 91% of its CPU was BackfillLibraryDiscographies → upsertBatch, with four of its six workers parked in sql.(*DB).conn. Jobs now run in submission order on one goroutine per component, each carrying its own snapshot. A job must not touch the component's fields — it holds no lock and the state has moved on — which is why persistTracks clones. SaveState still flushes and waits, because that is the one caller for which the row has to exist on return. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UDCbcCZQepnpSQYJ6SxxZm	2026-08-14 13:33:29 -04:00