61d549a9d5b936a58e99a95d51b8a41ed7d0b31f
100
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
cc9df4004c |
Merge pull request 'Surface a confident autotag match on the album page' (#121) from feat/28-autotag-match-on-album into main
The album page says when the autotagger has a confident match for what you are looking at, and can apply it. The tier behind "confident" is one name shared with strict auto-accept (#90), and the lookup costs no MusicBrainz request. Closes #28 |
||
|
|
b5d70ac1cd |
feat(explore): offer the autotag match on the album page
The complaint was having to notice the metadata was missing, then go and hunt the album down on the Autotag page. The album page now says it while you are looking at the thing: "MusicBrainz has a match for this album: <release> by <artist>", with Apply tags and Review in Autotag. Four things about it are load-bearing. **Applying is offered only where it would do the whole album.** A tagging group is a folder, so a multi-disc album is several, and one button that applied to the best-scoring group would leave the album holding a mix of old and new tags — the exact case the app's Blocking notification level exists for. `groupCount` is the test, and the answer there is review rather than apply. **It rewrites files, so it asks.** `confirmAction()` with an impact line that says it cannot be undone and that nothing is moved or deleted, because "rewrites your files" reads worse than it is. The apply goes through `ApplyAsync`, the registered-job path, so progress belongs to the jobs indicator and this page does not grow a second one — what it owes the user is the acknowledgement, because the button is here. The suggestion clears itself on success rather than inviting a second click while the job runs. **The banner does not quote a percentage.** The backend has a score and deliberately keeps it out of the sentence: 0.95 reads as a probability and is not one. Which release it is, is the part a person can judge. **"Review in Autotag" lands on that album.** The queue is sorted by score so the intended folder is often near the top, and "often" is a link that sometimes opens a different album. Autotag is a cached primary view, so there is no construction to hand a payload to: the request goes on as an attribute and the view *consumes* it, or every later visit would reopen a folder the user finished with long ago. `ICON_AUTOTAG` joins the vocabulary at the same time, on the rule `ICON_PLAYLIST` was chosen by — an icon names the noun it acts on, so a suggestion pointing at Autotag wears the Autotag destination's own mark. It was written inline in the sidebar; two call sites is where a name stops being one component's detail, so the sweep governs it now. Verified against the running app with a staged match: the banner, the confirm dialog's wording, and the navigation landing on the right folder with the attribute consumed. Closes #28 |
||
|
|
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. |
||
|
|
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. |
||
|
|
21b303ba7c |
fix(ui): stop a closing dialog answering the next question
`confirm-dialog` is one singleton for every confirmation in the app, and `wa-dialog` reports its close asynchronously: `open = false` starts an animation and `wa-hide` arrives after it. So a hide belonging to a question already answered can land after the *next* question has opened, and cancel it — the user is asked something, the dialog vanishes on its own, and the call site is told they said no. Each ask now carries an id. `close` ignores an id that no longer names the question on screen, the button handlers pass none (they always mean the current one), and only the `wa-hide` handler carries one, because only `wa-hide` can arrive late. Found by writing two `confirmAction()` tests in one file: the second could not be accepted at all, because the first one's hide had cancelled it before the click landed. Reaching it in the app needs two confirmations close together, which the album page's "Apply tags" makes possible. |
||
|
|
9375f25629 |
Merge pull request 'Demote the album page version selector to a disclosure' (#120) from feat/17-demote-version-selector into main
Choosing a pressing is a repair job, not the album page's headline. The selector is a collapsed disclosure below the tracklist; the two unguarded blocks that shared its slot are gone, and the catalog error now belongs to the list that is missing because of it. Closes #17 |
||
|
|
905654cc84 |
feat(explore): demote the album page's version selector to a disclosure
Choosing which pressing you are looking at is an advanced, metadata-repair task, and it sat directly above the tracklist with a heading, a `<select>` and a paragraph explaining how our clustering picks a "standard version" by weighing release count, status and date. That is a sentence about our own heuristic in the most valuable space on the page. It is now "Other versions of this album (N)" below the tracklist: a real `<button aria-expanded aria-controls>` inside the heading that names the section, with the body rendered unconditionally and toggled with `hidden`, because `aria-controls` has to name an element that is in the DOM. Both rules are `config-section`'s rather than new ones. It is demoted, not removed — matching the wrong release is a real problem and this is how it gets fixed. **Two more blocks shared that slot and neither was guarded.** The selector at least had `distinctTracklistCount() <= 1`; the `Versions / Loading releases…` spinner and the `Versions / <error>` block did not, so both took the primary position on every album regardless of whether there was ever going to be a choice. The spinner said what `renderTracklist` was already saying about the same fetch, so it is gone. The error was the one `catalog-scope-notice` shows at the top of the page with a retry — every path that sets `errorReleases` also sets `catalogFailed`, the only route to `unavailable`. That error is what made this a rewrite rather than a move. `renderTracklist` returned `nothing` on `errorReleases` and leaned on the selector's own block to have said it, and a control inside a collapsed disclosure cannot be a page's error surface. The failure belongs to the list that is missing because of it, so that is where it is drawn. **What must not be lost is which version is on screen.** The default is what the header already describes, so saying it on every album would be this issue's own complaint one size smaller. `defaultVersionKey` is the test: a line appears above the tracklist only once someone has chosen another, naming it and offering the way back. The ★ and the words "in your library" survive unchanged inside the panel, and the panel does not close when the selection changes — a panel that shuts on use cannot be used twice. The `<select>` also loses an `aria-label` of "Select release version" that outranked its own visible `<label>Version</label>`, which is a label not in the name. Verified against the running app as well as the suite: the collapsed page, the open panel, a chosen version and 390px width all read correctly, and the shell still measures 390 in a 390 viewport. Closes #17 |
||
|
|
219fa3c615 |
Merge pull request 'Make it obvious everywhere when you are looking at things you do not own' (#117) from feat/38-ownership-visibility into main
Owned is plain; unowned is dimmed, named and requestable; a partly-held album says how partly. Ownership is a file (`localId`), never the `in_library` ratchet. Closes #38 |
||
|
|
c4e055ce51 |
docs: write down which of the two ownership columns to read
The `localId` / `inLibrary` choice outlives #38 — every future catalog surface has to make it, and the code read them as an OR at eight call sites precisely because nothing said they were different kinds of thing. CLAUDE.md gets the rule and its four load-bearing details; NOTES.md gets the measurement, the card that used both answers at once, and the alternative that was rejected. |
||
|
|
10eca353ab |
fix(explore): gate playback on the same answer the row is drawn from
Two play paths still accepted `inLibrary`, so a row drawn dimmed and `aria-disabled` by the new rule would still attempt to play and fail with "this track could not be found in your library" — the disagreement this pass exists to remove, one layer down from the badge. |
||
|
|
88fc50afb8 |
feat(explore): mark what is not owned, everywhere it can be shown
`explore-album-details` had the rule right for one tracklist and nothing else did: Explore's cards, `top-results-row` and the artist page's three card shapes all mixed owned and unowned with a small badge as the only difference, and drew a green tick on the *common* case — which is the treatment that tracklist's own green ticks were removed for. `utils/ownership.ts` is the rule written once, so eight call sites stop each holding their own version: - owned is plain, and draws no badge at all; - unowned is dimmed *and* says so in its accessible name, because dimming is a colour and cannot be the only signal; - a partly-held album says how partly. **Ownership is a file, and `localId` is the flag that says so.** The album page answers with `filePaths`, a real file per displayed track; a card grid cannot afford that and does not need to, because `local_*_id` is built by queries that all join `audio_files` and cleared by a prune whose existence test is a file test in every case. `inLibrary` is written by the same pass, so the two agree in a healthy database — but it is a one-way ratchet (`MAX(in_library, excluded)`) whose only clearing pass is gated on a non-null local id, so it cannot be un-set on its own. Where they already diverged was the client. Both `explore-view` and `explore-artist-details` kept a `libraryMBIDs` set that accumulated every MBID ever seen with `inLibrary` and cleared it never, in views that never unmount. Both are deleted. And one card answered the question twice and got two answers: `renderReleaseMenuItems` gates Play on `localId > 0` while the badge and `albumTarget.owned` used `inLibrary`, so an album with the flag and no local row drew a tick saying it was in your library, offered no Play, and — the request item being gated on *not* owned — offered no way to ask for it either. The count comes from `completenessStore`, shaped like `credit-store`: `request()` is per-card and coalesces a screenful into one `GetAlbumsCompleteness`, absence is cached as an answer, and the whole cache is dropped on a scan, a retag or a removal rather than aged. `aria-disabled` goes on rows that cannot be activated and deliberately not on cards: an unowned card still navigates to the catalog page for it, which is a perfectly good thing to do with something you do not own. Audited and unchanged: `home-view`, `downloads-view`, `cover-grid`, `artist-details` and `genre-details` cannot show catalog content, so everything on them is owned and "owned is plain" is already what they do. The album page's own header badge stays, because that page is about one entity and the badge is its answer rather than a mark on one of many. Closes #38 |
||
|
|
19c68d73a7 |
fix(ui): keep the count in a partial badge that can act
A control is named after what activating it does, so an actionable badge said "Request album X" — and `partial` is actionable, because an album you hold nine of twelve tracks of has three left to ask for. That made the one state the ring exists for the one state whose name did not mention it. The argument the `partial` branch already carries does not stop applying because the badge became clickable: a ring says "some" to a sighted user and nothing to anyone else. The name is now the action and the count. |
||
|
|
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. |
||
|
|
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. |
||
|
|
fc99d9e0d7 |
Merge pull request 'Make a release a shipment rather than a merge' (#116) from ci/115-manual-release into main
Closes #115 |
||
|
|
90f1239fba |
ci: make a release a shipment rather than a merge
release.yml fired on every push to main, so the trigger was "a PR was merged" and nothing else decided. That is a version per unit of *work* rather than per *shipment*: eight releases in twenty-two hours, v0.0.1 through v0.3.1, for one session -- each fanning out to four publishers on a runner with capacity 1, so roughly forty packaging jobs shipped three issues while ordinary PR CI queued behind them. pacman, Homebrew and Obtainium see every one. The push trigger is gone and workflow_dispatch, which was already there and already worked, is the whole mechanism. Nothing else had to change to batch releases, because semantic-release already reads every commit since the last tag: five fixes and two feats become one minor release with all seven in the notes. Release frequency was only ever how often this file fired. This is the rule index-artifact.yml states and is the other instance of: a job that mutates state which cannot be rebuilt in ten minutes is triggered deliberately, not by a push. A release here is a tag, a Gitea release, an Arch package, a Homebrew formula, a signed APK and desktop assets -- and an Android version going backwards costs the user their library. `dry_run` is what makes a manual trigger usable: the point of pulling a lever by hand is being able to look first, so the input runs semantic-release --dry-run -- the version and the notes, no tag, no release, no publishers. Anything but the literal string "true" releases for real, because a typo in a dispatch box must not silently turn a shipment into a green no-op. Two alternatives were considered and rejected, both recorded on the issue. A `beta` integration branch relocates the trigger rather than removing one: it needs a second protected branch carrying the same required checks, and it *adds* a full check + e2e run per batch on the very runner whose queue is the complaint. A schedule batches without anyone having to remember, but puts the decision back on a timer, which is the thing being removed. Closes #115 |
||
|
|
b2fe1cb1e0 |
ci: skip a prerelease tag in all four publishers
Their trigger is `v*`, which matches `v0.4.0-beta.1`. They guarded `v0.0.0` -- the version floor -- and nothing else, so the first prerelease tag would have published a beta everywhere. Nothing produces one today. The guard is here because the thing that would is `prerelease: true` in .releaserc.yml, a one-line change whose blast radius is four public channels and which nothing in those four files mentions. That is the same argument release.yml's `chore(release):` guard is kept on: cheap, against something a future edit turns on somewhere else entirely. android-apk is the worst of the four twice over. Its APK goes to the *generic* registry, which is readable without credentials so Obtainium can poll a plain URL, so a beta would be offered to every device on it. And its versionCode maths splits on dots: it would read "1" out of "0-beta" and produce a wrong number rather than a failed build, which matters because Android orders releases by that integer and refuses anything not greater than what is installed. Each is a clean skip rather than a failure, matching the v0.0.0 guard beside it: a red run against a tag that was never meant to ship is noise. |
||
|
|
065a879190 |
Merge pull request 'Give the icons one vocabulary and sweep the call sites' (#114) from feat/34-icon-language into main
Release / release (push) Successful in 32s
Build & publish Arch package / arch-package (push) Successful in 2m35s
Attach the desktop build to the release / linux (push) Successful in 56s
Sync Homebrew formula / sync-formula (push) Successful in 6s
CI / e2e (push) Successful in 6m17s
CI / check (push) Successful in 3m8s
Build & publish the Android APK / apk (push) Successful in 1m29s
Closes #34 |
||
|
|
89882b4863 |
refactor(ui): give the icons one vocabulary and sweep the call sites
`plus` meant "add to the queue", "add to a playlist", "make a new playlist" and "you do not own this" -- the first two adjacent in the same context menu, so two neighbouring items were the same glyph doing different things. `list` meant the queue (the button that opens it), the Playlists destination, and adding to the queue in `queue-panel` alone. Two icons carrying seven meanings is not a vocabulary, and nothing catches it: a wrong-but-real icon renders perfectly. `utils/icon-language.ts` is the table, beside `library-status.ts` as the issue suggested. The rule it is built on is that an icon names the **noun** it acts on, not the verb: "add to queue" and "add to playlist" are one verb on two nouns, so the noun is what differs -- which is why adding to a playlist wears the Playlists destination's own icon, and why the queue took `bars-staggered` and stopped wearing Playlists'. `plus` keeps the one meaning it is unambiguous about, making something that is not there yet, which covers New Playlist and the drop zones. `bars-staggered` is the only new glyph, vendored through names.txt and fetch-icons.mjs after confirming it is in Font Awesome **Free** 7.3.1. Two things this found rather than changed: - The request toggle's outline/solid pair was already in the app and already right -- `explore-album-details`'s "Request this" button has used `regular/bookmark` -> `solid/bookmark` since it was written -- while the badge forty pixels away showed a **plus** for the same state. That is `utils/library-status.ts`'s fault one layer down: it made the two surfaces agree on what wanting *means* and left them disagreeing on what it looks like. - `explore-artist-details`'s Follow button was `bookmark-check`, which is Font Awesome **Pro** and has never been bundled, so it has drawn the missing-icon fallback -- a circled question mark -- for every followed artist since it was written. `requested-badge.spec.ts` was written for exactly this bug on the album button and says so in its docstring; this is the same bug one component over, still live, because `offline-icons.spec.ts` sweeps `__yjIconMisses` and no spec had ever followed an artist. So the test does what reaching the state cannot. `icon-language.test.ts` reads every `src/**/*.ts` as raw text and fails on a governed name written outside the table, and separately asserts every `ICON_*` is a *bundled* name -- which is what makes a Pro name a failing test rather than a runtime report from a state something has to reach first. Its first assertion is that it read any source at all, because a sweep over an empty glob passes. `chrome.test.ts` asserted `['check', 'bookmark', 'plus']` and so pinned the badge's glyphs against the vocabulary they were meant to follow; it names them from the table now, and keeps the assertion that the three differ, which is the property the states actually need. Downloads keeps the solid bookmark on purpose. That is one word twice, not two words: the badge says the entity is on your list and the nav item is that list. Closes #34 |
||
|
|
18a08daa91 |
Merge pull request 'Let the album page be asked for the whole tracklist' (#113) from feat/7-full-tracklist-toggle into main
Release / release (push) Successful in 33s
Build & publish Arch package / arch-package (push) Successful in 2m45s
Attach the desktop build to the release / linux (push) Successful in 1m2s
Sync Homebrew formula / sync-formula (push) Successful in 7s
CI / e2e (push) Successful in 6m18s
CI / check (push) Successful in 2m25s
Build & publish the Android APK / apk (push) Successful in 1m34s
Closes #7 |
||
|
|
aa59773d22 |
feat(explore): let the album page be asked for the whole tracklist
An album the user holds part of showed only the tracks on disk, with nothing to say the rest existed. The page could already draw the full release with the missing rows dimmed -- it just could not be asked: the automatic rule fires on `completeness.known`, which depends on the files declaring a per-disc total, or failing that on the catalog's own `total_tracks`. Neither reaches most albums. #16 fixed the first input for anything tagged from now on, and the second is worse than it looks: the published artifact is from 2026-08-10 and the column landed on 08-16, so `completenessAnswer()`'s catalog fallback answers 0 for every user until the index job republishes. Measured, and noted on #88, which is the publish that carries it. So the control is explicit. A "Show the whole album" switch flips the synthetic "Your Library" entry between the local files and the release, which is the same rendering, reached deliberately rather than inferred. Three things about it are load-bearing: - `showFullTracklist` is a tri-state, `null` meaning "follow the automatic rule". The rule is right when it fires, and the switch has to agree with the page it is sitting on rather than starting out contradicting it -- a plain boolean would need its default recomputed every time the completeness answer moved underneath it. The user outranks the rule in both directions. - `fullReleaseCluster()` falls back to the highest-scoring cluster. `findLibraryCluster` is a guess over the `inLibrary` flags and returns nothing at all when none are set, which is exactly the untagged library this exists for -- without the fallback the control would be absent precisely where it is needed. The sublabel names the release either way rather than leaving the user to wonder whose tracklist they are reading. - It appears only where it can change what is on screen: against the library entry, with a release to switch to, and only when the two tracklists differ. A complete album's release has the same rows as its files, so the switch would redraw the same list and read as broken -- the same test the version dropdown one section up already answers. The accessible name is asserted rather than assumed, through the browser's own computation. `wa-switch` happens to get it right, and for a third reason again: its `<input role="switch">` sits inside a native `<label>` that also holds the `<slot>`, so the name is computed across the flattened tree from light-DOM text. This app has shipped the opposite twice. Closes #7 |
||
|
|
a4777f26b6 |
Merge pull request 'Declare the track and disc totals when tagging' (#105) from fix/16-tagwriter-totals into main
Release / release (push) Successful in 30s
CI / e2e (push) Successful in 6m8s
CI / check (push) Successful in 2m23s
Build & publish the Android APK / apk (push) Successful in 1m25s
Build & publish Arch package / arch-package (push) Successful in 2m31s
Attach the desktop build to the release / linux (push) Successful in 54s
Sync Homebrew formula / sync-formula (push) Successful in 6s
Closes #16 |
||
|
|
92faa9741b | Merge branch 'main' into fix/16-tagwriter-totals | ||
|
|
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 |
||
|
|
3c3197df4b |
Small-fix batch: ten issues from the desktop backlog (#83)
Release / release (push) Successful in 33s
CI / e2e (push) Successful in 6m10s
CI / check (push) Successful in 2m26s
Build & publish the Android APK / apk (push) Successful in 1m26s
Build & publish Arch package / arch-package (push) Successful in 2m36s
Attach the desktop build to the release / linux (push) Successful in 56s
Sync Homebrew formula / sync-formula (push) Successful in 6s
Rolls up #82 (@yonlu) and #74, #75, #76, #77, #78, #79, #81 as one push, so the batch cuts one release rather than eight. Closes #9, #13, #14, #19, #26, #29, #33, #35, #37, #41. |
||
|
|
e16bd245bd | Merge remote-tracking branch 'origin/fix/queue-toggle-state' into integration/small-fixes | ||
|
|
887a9324b4 | Merge remote-tracking branch 'origin/fix/drag-count-badge' into integration/small-fixes | ||
|
|
fcb484ead5 | Merge remote-tracking branch 'origin/fix/album-card-year' into integration/small-fixes | ||
|
|
48de41cd69 | Merge remote-tracking branch 'origin/fix/album-tracklist-heading' into integration/small-fixes | ||
|
|
66a6ee63ab | Merge remote-tracking branch 'origin/fix/seek-bar-clock-width' into integration/small-fixes | ||
|
|
10660c8168 | Merge remote-tracking branch 'origin/fix/wanted-without-client' into integration/small-fixes | ||
|
|
441b67daaa | Merge remote-tracking branch 'origin/fix/album-track-request-badge' into integration/small-fixes | ||
|
|
026f26bdf6 | Merge remote-tracking branch 'origin/fix/small-issue-batch' into integration/small-fixes | ||
|
|
73dc80bdc9 |
fix(explore): stop hiding the request badge until the row is hovered
The badge on a row you do not own was transparent until the row was hovered or focused. That rule was inherited from the green ticks it replaced, and it does not survive the reason those went: a tick marked the *common* case, while this marks the rows that are not here. A mark on the exception is the information on this page, and one that appears only under the pointer cannot be seen, counted, or reached by anyone driving the app with a finger. The repaint half of #33 is fixed in #82; this is only the visibility, rebased to leave that alone. Refs #33 |
||
|
|
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 |
||
|
|
a2ff0aed4c |
fix(ui): make the queue button say whether the queue is open
It looked identical in both states, so the only way to tell what pressing it would do was to look at the other side of the window and infer it -- and for anyone not looking there was nothing to infer from: no aria-expanded, no aria-controls, no drawn state. The state is reflected *from the panel* rather than kept beside the click. This button is not the only thing that opens the queue -- now-playing-view sets the same attribute, because it hides the bar the button lives in -- so a flag maintained by the click handler would be right until something else opened the panel and then quietly wrong. The panel's `open` attribute stays the one fact; a MutationObserver reflects it. Refs #26 |
||
|
|
12e75ee24c |
feat(ui): badge an album drag with how many tracks it carries
Dragging an album to the queue put its cover under the cursor and said nothing about how much that was -- an album is 1 track or 30 and the thumbnail is the same picture either way, so the one number the drop is about was the one thing the drag did not show. Every other drag in the app already says it; this was the exception, because it had a picture to show instead. A count of 1 draws no badge: "1" over a single cover is noise, and the absence reads clearly beside a badge that only appears above one. The badge sits inside the cover's box rather than overhanging it, because setDragImage snapshots the element and anything outside it risks being clipped -- while padding the box instead would move the cover away from the cursor. Refs #19 |
||
|
|
792e87298b |
fix(ui): stop the album grid eating the year it was sorted by
The year sat inside the same ellipsis box as the title, so it was the first thing truncation took: a card wide enough for a long album name never showed its year, and browsing the grid *by year* showed years only for the albums with short names. The sort said one thing and the cards showed another. Title and year are now a flex row where only the title gives way. A row rather than a second line, because the card's height is what the virtualizer measures rows by. Refs #29 |
||
|
|
266e7032dd |
fix(explore): stop labelling the album tracklist "TRACKLIST"
A list of numbered titles with durations, under the album's cover, was the one thing on the page carrying a word above it saying what it is. What goes is the ink and not the element: the section is a landmark and the page's heading structure runs through it, so the h3 stays and is clipped the way sr-only clips -- never display:none, which would take it out of the accessibility tree along with the layout. Refs #9 |
||
|
|
d6b48fb3ac |
fix(player): stop the seek bar resizing as its clocks count
Two different things moved it and they need different answers. Digits in a proportional font are different widths, so 1:11 is narrower than 4:08 and the bar breathed once a second -- tabular figures fix that. The character *count* changes too, at the hundredth minute and whenever the right-hand clock is toggled to remaining and grows a minus sign, which a figure width cannot fix -- so each clock reserves the widest string this track can put in it. The budget is per track rather than a constant: reserving six characters on every track would push the slider in by a character at each end to buy nothing. Measured in the component tier: 4.5px of drift across three positions before, none after. Refs #13 |
||
|
|
e1c07438e9 | docs: record what shipping the release pipeline taught us (#4) | ||
|
|
6e563f3846 |
docs: record what shipping the release pipeline taught us
Moves plan 017 to completed with a recap, and lifts the three findings that generalise into NOTES.md: a preset major that renders empty notes with everything green, a 403 that looks like branch protection and is a token scope, and tag-triggered workflows running the tagged commit's own definitions. |
||
|
|
186f6a5839 |
fix(release): seed the version floor on the parent, not on HEAD (#3)
Release / release (push) Successful in 32s
CI / e2e (push) Successful in 6m7s
CI / check (push) Successful in 2m29s
Build & publish the Android APK / apk (push) Successful in 1m24s
Build & publish Arch package / arch-package (push) Successful in 2m26s
Attach the desktop build to the release / linux (push) Successful in 2m29s
Sync Homebrew formula / sync-formula (push) Successful in 6s
|
||
|
|
786d9c6110 |
fix(release): seed the version floor on the parent, not on HEAD
The floor tag marks what has already been released, so tagging the
commit being pushed leaves nothing between the floor and HEAD --
semantic-release then correctly reports there is nothing to release.
That is what the first run did: it seeded v0.0.0 on the merge commit
itself and cut no release.
HEAD^ is the first parent, so on a merge commit it is main as it was
before the merge and everything the merge brought in is releasable.
The tag has been moved to
|
||
|
|
0019310ca4 |
ci(release): cut releases from main automatically (#2)
Implements .planning/plans/active/017-release-automation.md. Merges to main now compute the version from Conventional Commits, cut the tag and the Gitea release, and the four v* workflows publish and attach their artifacts. First release is v0.0.1. |
||
|
|
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. |
||
|
|
37e3373db9 |
docs: correct the workflow counts these comments name
Adding release.yml and desktop-assets.yml made 'the three workflows a tag fires' wrong in three files that each said it slightly differently. |
||
|
|
9ce79ee416 |
ci(release): release from a branch, not a detached HEAD
semantic-release resolves the release branch and then pushes a commit and a tag to it, so a local branch named main is a better starting point than the --detach the other five workflows use. Still pinned to the pushed commit rather than to whatever main points at by the time the container starts. The floor tag falls back to the PAT when GITEA_TOKEN is unset, which is safe rather than merely convenient: all four publishers skip v0.0.0 explicitly, so the worst case is four jobs that start and immediately say there is nothing to build. |
||
|
|
b3a0814f24 |
docs: describe the release pipeline where the claims used to be wrong
CLAUDE.md said .releaserc.yml was a config nothing ran and that there were five workflows; both stop being true with this branch. The CI section now names release.yml as the entry point and records the four things in it that are load-bearing, including the two silent failure modes worth pinning against. packaging/homebrew/README.md and docs/android-release.md say where a user would actually look that upgrading from 1.x needs a reinstall -- Homebrew offers nothing silently, and Android refuses outright. |
||
|
|
2c576fa1e8 |
ci(release): attach the Linux, Arch and Android builds to the release
A release page with nothing to download is one nobody can use. The Arch package and the APK are already built and merely go unattached; the plain Linux binary is new, and is what answers 'get the latest version' without a package manager. scripts/release-asset.sh waits for the release to exist first. semantic-release pushes the tag in prepare and creates the release in publish, so the tag push that starts these workflows happens before there is an id to upload to -- and a capacity-1 runner serialises that into working by accident, which is the worst kind of bug. macOS is absent because it cannot be built here: GOOS=darwin CGO_ENABLED=0 fails at wails/v3/pkg/mac, the darwin backend being Objective-C behind cgo. Homebrew builds from source on the user's Mac and stays the macOS channel. Windows cross-compiles cleanly and is still withheld: no build of it has ever been run. All three skip v0.0.0, which is semantic-release's version floor rather than a shipment. |
||
|
|
544dbdb4db |
fix(packaging): stop publishing an Arch package on every merge to main
arch-package.yml ran on push to main and took its version from `git describe`, so the pacman registry accumulated one package per merge and not one of them corresponded to a version a user could be told to install. It builds the tag release.yml cuts instead. pkgver's literal drops to 0.0.1 with it. That is a downgrade from the 1.x already in the registry, so pacman offers no upgrade and an existing install has to be removed once; epoch=1 would have avoided that and is declined in a comment, because an epoch can never be removed again. |
||
|
|
087eb77875 |
ci(release): cut a release from main with semantic-release
The config has been sitting in .releaserc.yml complete and uninvoked; this is the workflow that runs it, and the one Gitea-shaped adaptation it needs. @semantic-release/github speaks GitHub's API, not Gitea's /api/v1, so @semantic-release/exec calls scripts/gitea-release.sh instead. That script reads the notes out of CHANGELOG.md rather than taking them as an argument: release notes are rendered commit messages, so interpolating the notes into a shell command would be an injection whose input is the commit log. The tag is pushed with a user PAT because Gitea does not start a workflow from a ref pushed by a workflow's own token, and the three publishing workflows are keyed on it. |
||
|
|
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.
|
||
|
|
52cbef27c4 |
docs: name the guard that covers every cache table
The bullet added with the credit work names `TestTheCatalogSurvivesAStaleShape`, which pins the table and shape that failed. The general guard landed the same day and is the one that covers a table nobody remembered -- flipping the policy back fails it on five, including both artist-credit tables. |
||
|
|
c03c0b8ec4 |
test(database): the next destructive repair fails a test, not a volume
The fix for the dropped catalog pins one table in one wrong shape, which is the failure that happened. What cost the rebuild was more general: a destructive repair added at `database.NewDB` -- the chokepoint every binary in this project shares -- without asking which binary it runs in. The next one will have a different name and a different reason. So `TestNoCacheTableIsRetiredHere` asserts the outcome instead: put every `datamap` Cache table into a shape the schema has moved past, open the database the way cmd/indexbuild does, and require all of them to still be there. Driving it from `datamap.ByKind` is what makes it cover tables nobody remembered -- flipping the policy back fails on five, including the two artist-credit tables added the same day, where the existing test fails on one. It asserts the rows survive too, because SQLite does an implicit DELETE before a DROP and a repair that recreated the table would look identical. And it accepts an error from `NewDB`, because that is the documented trade: loud is recoverable, gone is not. `scripts/index-cache-snapshot.sh` covers the half no test can reach. The volume holds the only copy of a catalog that costs hours of someone else's bandwidth to re-derive. `VACUUM INTO` rather than `cp`, since a byte copy of a live SQLite file is a corrupt file of plausible size; the resumable staging directory is skipped; and each snapshot is reopened and asked for its catalog row count before anything is rotated out. A corrupt source and an empty catalog were both exercised: each exits non-zero, removes its own output, and leaves the previous snapshots alone. docs/index-cache.md is the restore, and the reason to bother: a restored snapshot resolves to `refresh` and folds in the listens since, which is minutes against the 3-23h this rebuild has been estimating. |
||
|
|
1c4d6ca9a1 |
ci: stop booking three hours of runner on every push
The catalog this job derives was dropped by the stale-shape repair (see `fix(database): never retire the catalog the index build derives`, which prevents a recurrence but cannot undo it), so `mode=auto` now resolves to a full ~205 GB import from the dumps. That import runs on every push to main with a 3h budget, on a runner of capacity 1 -- so ordinary CI has been queuing behind it since the merge, and each further push books another three hours. The damage is the repetition, not the single job. The `push` trigger is commented out until a run reports `complete=true`. The weekly cron and workflow_dispatch still resume the build, which is all it needs: indexbuild picks up from its checkpoint, so nothing already imported is re-fetched. Restoring the two commented lines is the entire revert, and the comment beside them says so. NOTES.md carries the incident, including the two things worth changing regardless: a destructive repair running inside `database.NewDB` has to ask which binary it is in, and the only copy of a 205 GB derived asset is a single Docker volume with no snapshot. |
||
|
|
d0250a2133 |
docs: confirm the phone track list on the phone
Build & publish Arch package / arch-package (push) Successful in 2m32s
Search index maintenance / maintain-index (push) Successful in 7s
CI / check (push) Successful in 2m26s
CI / e2e (push) Successful in 6m12s
Build & publish the Android APK / apk (push) Successful in 1m46s
Sync Homebrew formula / sync-formula (push) Successful in 7s
The arrangement and the width fix, measured on the device with the build installed rather than at the same viewport in a browser: `24px 304px 80px`, 52px rows, no header, the title untruncated, no overflow. Same numbers both places, which is why both were measured. |
||
|
|
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. |
||
|
|
2c78b58207 |
feat(ui): the track list a phone can read
B2 phase 4, and the last of it. Measured on the device: at 424 CSS px the four configured columns fit the row *exactly* -- `--grid-cols` came out `24px 102px 101px 101px 80px` -- and not one of them fit its content, with "Duration" too narrow for its own header. The columns were never too wide; there were too many of them. So a phone draws `titleArtist` (the title with the artist under it, across the row's whole width) plus the duration, and drops the column headers and the resize handles, which are a click-to-sort and a drag with no touch equivalent. It is a **column set, not a second row template**: the row, its delegated events, the selection semantics, the playing marker and the virtualizer never learn anything changed, because from their side only the number of columns did. Three rules come with it. The row height is in two places (`PHONE_ROW_HEIGHT` and the CSS rule) and must agree, since the virtualizer positions rows from that number and a taller row overlaps its neighbour. What is drawn and what can be sorted are different questions, so the sort list is built from `configuredColumns` -- a phone has no headers either, and building it from the drawn columns would leave it able to sort by title and duration alone. And a phone's column widths are neither loaded nor saved. That third rule is the bug the device found with the arrangement already passing five component tests and five e2e specs at the phone's own viewport. `loadColumnWidths` is keyed by column *id* and fills a gap with `MIN_COLUMN_WIDTH`, so the stacked column -- which nothing can ever have saved a width for -- came out at 148px beside a duration column of 236. The mirror image was worse and unreachable from a phone at all: saving would have written those widths back under the same ids, replacing the width the user dragged on a desktop. The specs asserted shape, and the fault depended on what `localStorage` held for a different column set; the unit test now carries that map as a fixture. Verified: 809 component tests, 112 e2e specs, and on the phone at 424x439 -- `24px 304px 80px`, 52px rows, no truncation, no overflow. One full e2e run of three saw an unrelated autotag keypress spec flake and pass on retry. |
||
|
|
a9852c18a0 |
docs: the device answered both open questions, and neither as expected
Both faults reported from the phone are now measured rather than inferred, with the installed build and current main compared on the same device. "The controls are off screen" was literal and already fixed: the installed build predates B2 phase 2, so its player bar still carried the seek bar and volume at 424px and the transport ran past the right edge. Current main measures no horizontal overflow and the controls at 200..380 inside 424, on the phone's own engine. "No icons" was my own screenshot: taken six seconds after a cold start, before the icon fetches landed. On the settled app every icon paints, and the earlier black `fill` was the svg root rather than the path that carries `fill="currentColor"`. Two conclusions from one misread node, both corrected. Chrome 113's missing Popover API does not break the menus, which was the standing worry: a long-press opens the real panel with seven items, positioned and painted -- so long-press is now verified on hardware over a 1,744-track library, not just in a browser at a phone-shaped viewport. What the device does add is a measurement for phase 4: the track list's columns fit the host exactly and are simply too many for 424px. |
||
|
|
0bfa2136be |
feat(dev): ask the phone instead of looking at it
The device tier could only take a screenshot and read what Go chose to log, and a screenshot cannot tell a dropped CSS declaration from a missing asset. This adds the third thing: the page's own answer, from the engine that is really rendering it. `make android-screenshot` grabs the screen, `make android-inspect` forwards the WebView's devtools socket, and `make android-eval EXPR=...` evaluates in the real page. Four details are load-bearing. Only a `debuggable` build opens that socket, so the debug build type takes `applicationIdSuffix ".dev"` and installs *beside* the release app -- the two carry different signing certificates, and Android's only remedy for a changed certificate is an uninstall, which takes the user's library with it. Playwright cannot drive a WebView (`connectOverCDP` calls `Browser.setDownloadBehavior`, which it answers "Browser context management is not supported"), so the eval is raw CDP over Node's built-in WebSocket. The socket name carries the pid, so it is resolved per launch rather than written down. And `exec-out`, not `shell`, for the screenshot: a pty translates LF and corrupts the PNG. What it immediately established is why it was worth having. The phone renders in Chrome 113 at 424x439 CSS px -- two years behind every browser the other tiers use, with no Popover API and no relaxed CSS nesting -- so a spec passing at that viewport says nothing about the device, and two conclusions drawn from version numbers alone were wrong. Both are corrected in NOTES.md and the plan. |
||
|
|
b1cdef8769 |
docs: record what a phone said that no tier could
The first device run of the published APK, and the first runtime evidence any of the Android work has ever had -- A4 shipped entirely reasoned from source. It confirms A4 whole: playback survives the screen locking, and the transport notification appears with cover art, which settles four open questions at once (the service starts, the permission was granted and the notification is visible, the lock screen picks up the session, and art decoded from a MANAGE_EXTERNAL_STORAGE path by a service is readable -- the one nobody could argue from documentation). It also found the two faults fixed in the preceding commits, and the lesson worth keeping is why *those two*: both are things the platform adds rather than things the app draws. So the skill's Android tier now says to ask a device about system bars, the back gesture, focus and audio interruptions, permissions and the keyboard -- and not about layout, which the other five tiers already cover. |
||
|
|
d661836347 |
fix(android): keep the app out from under the system bars
Reported from the first device run: the playback controls are off screen. `targetSdk 35` is Android 15, which lays every app out edge-to-edge and ignores the deprecated `statusBarColor` and `navigationBarColor` the scaffold's theme still sets -- so a `match_parent` WebView draws the page's bottom band, which on a phone is the transport *and* the tab bar, underneath the gesture bar. `applyWindowInsets()` pads the container by `systemBars | displayCutout | ime` and returns the insets rather than consuming them, so the WebView is laid out inside them. The keyboard is in the mask because a search box the keyboard covers is the same bug one surface over. The window background goes black to match the app's own default ramp: that padding is what shows through, and a band of the scaffold's blue-grey above and below reads as the app failing to fill the screen. No tier we have can see this class of fault -- a browser viewport has no system bars, so `phone-shell.spec.ts` at 390x844 renders a shell that fits at the moment the device is clipping it. Verified only as far as the APK building; the insets need the next build on a phone. |
||
|
|
28eecf0a97 |
fix(ui): the Android back button had nowhere to go
Reported from the first device run: back does not navigate back in the app. The scaffold's `MainActivity.onBackPressed` asks `webView.canGoBack()` and finishes the activity otherwise -- and this app had never touched `history`, so that was false at every depth and back quit from anywhere. The fix is here rather than in Java, because the mechanism the scaffold already uses is the one we were failing to feed: a navigation is a history entry now, and `popstate` replays it. Nothing on the Android side changes, and the behaviour becomes assertable in a browser with `page.goBack()` instead of only on a phone. The entry keeps the same URL -- the app has no routes, and a path a reload cannot resolve is worse than none -- and carries the destination in its state. Two rules keep the stacks from disagreeing. The first navigation *replaces* the launch entry rather than pushing one, or every launch costs a back press before the app will close. And the in-app back buttons go through `history.back()` rather than popping a stack of their own: `navStack` is deleted, not kept alongside, because two stacks is precisely how a detail view's own button and the phone's gesture come to disagree about how far one press goes. The third spec pins that invariant. |
||
|
|
e8690476bd |
feat(ui): long-press opens the menus a right-click opens
Every context menu in the app opens from a `contextmenu` event, bound three different ways across six components -- delegated on a virtualizer, per row, per card. A phone has no right-click, so a phone reached none of them (plan 016 B2 phase 3). This is one document-capture listener installed once from `index.ts`, not six components' worth of touch handling: a touch that holds still for 500ms dispatches a synthetic `contextmenu` at the touch point, and every existing handler runs unchanged. A seam no component has to opt into is one no future component can forget. Four details are load-bearing, each a way the obvious version fails. The target is `composedPath()[0]`, not `elementFromPoint`, which stops at the outermost shadow host -- every menu here is bound inside one, so a host-targeted event reaches a delegated listener and no per-row one. A browser that fires its own long-press `contextmenu` (Chromium does; WebKit and the Android WebView vary) wins, and ours is told from theirs by identity rather than `isTrusted`: `isTrusted` works in the app and is untestable, which would leave the suppression path as the one thing with no coverage. And the click ending the gesture is swallowed, keyed on the gesture rather than a time window, or the first tap on the menu it just opened is eaten too. The e2e spec presses `.track-row`, not `[role="row"]`: the column header is a row too, and it is the first one -- a press on it is correctly ignored, which reads exactly like the gesture not working. |
||
|
|
7e0be8fa30 |
fix(indexexport): read an index older than the binary
`maintain-index` failed with
indexexport: copy rows: SQL logic error: no such column: total_tracks
three minutes into the one job that owns the ~205 GB checkpoint and
publishes the catalog every user downloads.
The cause is the exception that keeps that checkpoint alive. The job's
/cache is a real YJ_HOME that survives between runs, so its
explore_index is classified Cache and is deliberately *not* dropped and
recreated by cmd/indexbuild's schema repair -- which means a column
added to the schema afterwards is absent from it. total_tracks arrived
with the album-completeness work; the exporter selected it regardless.
The fix is the rule the importing side already follows.
artifactHasTotals exists because "adding a column to the importer's
SELECT is how you break every artifact already published"; the mirror
image, reading an index older than the binary, had no such guard.
sourceColumns asks pragma_table_info and selects a literal 0 when the
column is absent -- which is what that column already means by "the
catalog does not say", and what the app renders as unknown rather than
as incomplete. The artifact keeps every column, so an importer needs no
second shape.
The test reproduces the failure symptom first: with the fix removed it
fails with the CI message verbatim. Its own first version proved
nothing, though, and that is worth the comment it now carries --
`strings.Replace(catalogColumns, "total_tracks, ", …)` matches nothing,
because the list is formatted across lines and the name is followed by
a newline, so the "old" index was built with every current column.
|
||
|
|
1b05dde382 |
feat(ui): the full-screen now playing a phone needs
Plan 016 B2, phase 2. Phase 1 took the seek bar and the volume out of the phone's bottom bar -- 4px of height is not a thumb target, and a phone's volume belongs to its hardware keys -- and promised them a full-screen view. This is it, reached from a button over the mini player's cover art. **It composes the transport rather than reimplementing it.** The same `seek-bar`, `player-controls` and `volume-control` the desktop bar uses; a phone layout that copies them is a second transport to fix every bug in, and the seek bar in particular carries interpolation rules that took a plan of their own to get right. The seek bar thickens its own track below the breakpoint, in its own stylesheet, because the track size lives on a wa-slider inside its shadow root where a custom property from the host cannot reach. **It is a detail view, not a primary one.** It is somewhere you go and come back from, so index.ts pushes the current view and Back pops it -- which is also why it is not a fifth tab: a tab you cannot leave by pressing it again is not a tab. Two things came from reading a screenshot rather than from a failing test, and both were invisible to assertions that were individually correct. **The mini player was still under the full-screen view**, repeating it in 4em of an 844px phone. index.css hides the bottom bar while `#main-content[data-active-view="now-playing"]`, through `:has()` rather than a class toggled from index.ts, because the active view is already published as an attribute. That takes the queue button with it, so the view carries its own. **And phase 1's shell rules had never applied.** A media query adds no specificity, and the phone block sat above the plain rules it meant to override, so at 390px the header kept its 2em gutters (32px), its 16px gap and its 24px title, and the bottom bar kept a fixed 320px first column. Nothing failed: the shell fits because of `min-width: 0` and each component's own media query, which live in their own stylesheets and have no later rule to lose to -- so what was dead was exactly the cosmetic half no assertion looks at. The phone rules are one section at the end of the file now, and it says why it is last. Measured after: 12px, 8px, 17.6px, `154px 187px 33px`. |
||
|
|
29299d17da |
fix(dev): run the local e2e tier against the app CI runs
Two specs failed locally and passed in CI, which is the least useful direction for a disagreement to point. **`dev-headless.sh` was the only launcher not stubbing out the catalog.** `seed-sandbox.sh` and `ci.yml` both send `YJ_CORE_INDEX_URL` to a dead address; the dev launcher did not, so the app downloaded and built the real ~1M-row Explore catalog into the run's YJ_HOME and every local `make e2e` after that ran against a world CI never sees. Found by reading the failure screenshot: the spec had searched Explore for its fixture album and the page was full of real ones. It defaults to the dead address now and takes an explicit one for exploring by hand. **And the shared backend carries spec state between runs.** `explore-shelves` staged its catalog only `IfEmpty`, so one album row left behind by `requested-badge` satisfied that gate: the shelves were drawn from a single foreign row and the artist card the spec clicks did not exist. It failed on the *second* local run and passed on the first, and never in CI, where every run gets a fresh home. "Is the catalog empty" was the wrong question and "are my rows there" is the right one, so staging is unconditional (INSERT OR IGNORE keyed on the MBID) and the assertion moved from *this insert wrote a row* to *every fixture row is present*. That is both idempotent and stronger: an MBID that fails CHECK(length(mbid) = 16) is silently dropped by OR IGNORE, which the old per-insert count caught only on a cold catalog and the new one catches always. Verified by running the whole suite twice against one app: 97/3 before, 100 passed both times after. |
||
|
|
57fbbdf0d2 |
feat(ui): a shell a phone can be held in
Plan 016 B2, phase 1. Below 600px the grid drops its sidebar column, `bottom-nav` becomes the primary navigation, and the shell fits the viewport instead of scrolling sideways out of it. 600 rather than the sidebar's own 900, because 900 is a laptop and the answer there is a narrower sidebar, which is still a sidebar. Under 600 there is no room for one at all: 360px of viewport over a 200px nav is not a layout. **The tab bar is four destinations and a way to everything else.** Three to five is where touch targets stop being thumb-sized -- eleven over 360px is 32px each -- so the four are the ones plan 016's subset says a phone is for, and "More" opens the *existing* `app-sidebar` in a drawer rather than listing the destinations a second time. Two lists is two places to add the next view to. That reuse has a cost this found the hard way: a shared component brings its `data-testid`s with it, so rendering the drawer's sidebar unconditionally put a second `nav-home` (and ten siblings) in the DOM and **failed 30 existing specs** with "resolved to 2 elements" -- on a desktop viewport, where this element is `display: none` and the drawer can never open. It renders only while the drawer is open, and the component test asserts the absence, because the failure is invisible from inside the component and lands in files nobody touched. **What made the shell overflow was minimums, not padding.** Measured at 360px: the body was 652px wide, because a `min-width` in a flex row is a hard floor and a grid item's implicit minimum is its content. So `min-width: 0` on the boxes between the viewport and the content, and each component stands its own non-essential parts down in its *own* stylesheet -- search-bar's 200px floor, job-indicator's label (the visible one; the live region that announces it is untouched), audio-player's seek bar and volume. A media query inside a shadow root is answered by the viewport, so this is the component saying what it drops rather than the shell reaching in. Volume goes because the hardware keys own it on a phone, which is the same reason mediacontrols' Android handler implements no volume callback. Seeking goes because 4px is not a thumb target; it belongs to the full-screen now-playing view, which is the next phase. An existing spec therefore asserts the opposite of what it did: layout-overflow's 320px case used to require that the 464px behind `overflow: hidden` could be *scrolled to*, which was the remedy available while the shell had one layout. It reflows now -- 320px in a 320px viewport, exactly -- and reflow is what WCAG 1.4.10 asked for. |
||
|
|
df2e9ea777 |
docs: record what the Android work established and disproved
Section A of plan 016 is closed and B1 is decided, so the three tenses move together: CLAUDE.md for what mediacontrols now is, the skill for what to run, NOTES.md for what was measured and when. The entry worth reading is the one that disproves a claim written here earlier in the same session. Dropping x86_64 was expected to make make android-install fail with INSTALL_FAILED_NO_MATCHING_ABIS. Measured, it installs and launches: Google's google_apis x86_64 images carry arm64 translation (abilist = x86_64,arm64-v8a), so the loader maps lib/arm64/libwails.so and runs it. It dies before any of our code with SIGILL, and the disassembly names the reason exactly -- `mrs x0, ID_AA64ISAR0_EL1`, Go's internal/cpu reading the arm64 feature register at runtime init, which the translator does not implement. So no Go binary starts under it, and that is not a property of this app. Which closes the last plausible shortcut. There are now three distinct ways this app fails on an x86_64 Android -- seccomp on the x86_64 build, an unimplemented system register on the translated arm64 one, and a real device still unverified -- and none of them is a bug in it. A phone remains the only verification path. Plan 016 also carries the B2 scope, now decided rather than recommended: option 1's data model with option 2's surface. The phone gets home, library browse, now-playing-as-a-view, the queue, search and playlists; it does not get autotag, downloads, Explore or the 93-control Settings page, and each of those has a reason written beside it. One rule for the work: no view forks, because a phone template that copies a view's is two templates to fix every bug in. |
||
|
|
c99c8efa11 |
ci(android): tell a wrong password apart from a wrong keystore
The v1.5.0 run reported that the keystore did not open, and the diagnostics could not say why. They now clear the two causes that look identical to a wrong password. **A password pasted with its shell quotes** is two characters longer than the password and nothing in keytool's error says so. The step retries with the surrounding quotes stripped and, if *that* opens the keystore, says exactly that. It does not strip them and carry on: a password may legitimately contain a quote, so this reports a diagnosis rather than guessing at a fix. **A password that is right for a different keystore** is the other one, and it is the one currently in play -- the secret decodes to a valid 2280-byte PKCS12 and the password is the length the owner expects, which leaves "is this the keystore I have locally?" as the open question. The step prints the decoded file's sha256 so that is answerable by comparing one line against sha256sum. Hashing a certificate store gives nothing away. |
||
|
|
904786b941 |
fix(dev): the Android harness did not parse, and then chose any device
Two bugs, and the first had made every make android-* target dead since the commit that introduced it. **The script did not parse at all.** A case pattern read `*signatures do not match*)`, and `do` is a reserved word: bash rejects the *whole file*, so android-emulator, android-install, android-smoke and android-logs all died with "line 190: syntax error near unexpected token `do'" -- a message that points at a line nobody had reason to suspect, in a file that had been working. Quoting the inner words fixes it. A shell script only ever run by hand can carry a syntax error indefinitely; nothing in the pre-commit hooks runs bash -n. **A bare adb addresses whatever is attached.** With a second emulator present -- another project's, or this one's own corpse left `offline` by a previous run -- every adb call fails with "more than one device", and cmd_install reported that as "no device - run 'make android-emulator' first" *directly after* that had printed "waiting for boot ok". Which is the harness's own house rule broken: a failure that names the wrong cause is worse than one that names none. pick_device resolves ANDROID_SERIAL from ro.boot.qemu.avd_name before any device command. The AVD name is the identity because serials are assigned in boot order and change between runs; a caller's own ANDROID_SERIAL wins, and a single device that is not ours is taken as the target, since that is a phone and a phone is what this tier actually wants. Verified with both emulators running. |
||
|
|
b6651310ea |
build(android): drop the x86_64 ABI, which no Android can run
The fat APK's second half was 31 MB that cannot execute on any Android device. modernc.org/libc's Xlstat64 issues a raw lstat syscall on linux/amd64, and Android's seccomp policy forbids it because bionic never issues it, so the process takes SIGSYS the first time anything touches the database -- which for this app is startup. That is every x86_64 Android, x86 Chromebooks included, not merely the emulator. arm64 is structurally unaffected: the architecture has no lstat syscall at all, so modernc routes through fstatat. 27,059,130 bytes to 15,898,465, and one lib/ entry. Three places had to agree, and the third is what would have made this a silent no-op: abiFilters (what Gradle packages), android:package rather than package:fat (what Go *compiles* -- otherwise the library is still built and then discarded), and the native-code assertion in CI. That assertion is anchored, `native-code: 'arm64-v8a'$`, because without the anchor it also matches the fat APK's line and would pass on exactly the thing it exists to catch. Checked against a real artifact. Adding the ABI back, if modernc ever fixes Xlstat64, is those same three edits. |
||
|
|
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. |
||
|
|
ced537ecf2 | docs: record which Android blockers are now cleared | ||
|
|
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. |
||
|
|
78576b8da9 |
docs: assess what Android parity would take
Plan 015 shipped a pipeline; this is what stands between that and an app worth installing. Verified against the source and the generated manifest rather than guessed. Four blockers, and none of them is porting work. The manifest requests no storage or media permission at all, so the app can read no music -- and READ_MEDIA_AUDIO would not be enough, because it grants access through MediaStore while this app's whole model is absolute paths: audio_files.file_path is the primary key of ownership and every GetFilePathsBy... query exists to hand paths to the player. The first-run wizard calls DirectoryPicker, which Wails documents as returning an error on Android, and the wizard intercepts pointer events until a library exists, so the app is inert rather than merely empty. mpris_linux.go is compiled in, because android implies linux. And the scaffold's foreground service is typed dataSync rather than mediaPlayback, with no MediaSession and no audio focus, so playback dies at screen lock and there are no lock-screen controls. They are all the same question: is the Android app a librarian or a player? The desktop app is a librarian -- it scans folders, dedupes covers, rewrites tags on disk -- and that model rests on owning a filesystem, which is exactly what Android declines to give. So the plan argues that parity is the wrong target and lays out three coherent products instead, recommending a MediaStore-backed player. Four things are worth doing whatever is decided, and the highest information-per-minute one needs no code: run the published APK on a real phone. Nothing in sections A or B has been observed on Android, because the x86_64 emulator cannot run the app and emulator 37 refuses arm64 images on an x86_64 host. |
||
|
|
01706c6053 |
ci(android): say why the keystore did not open
"the keystore did not open — is ANDROID_KEYSTORE_PASSWORD right?" is a guess, and there are three quite different reasons behind it. The step distinguishes them now. **A secret pasted into a web form very often carries a trailing newline**, and a password is compared byte for byte, so the run failed with a password that was correct. Reproduced exactly: keytool rejects `Correct123\n` against a keystore whose password is `Correct123`. CR and LF are stripped from the password, the alias and the key password now, and the step says when that mattered. **A wrong alias failed a minute later, inside Gradle.** It defaults to `yellowjacket`, so any keystore created with another alias got there. The alias is checked up front and the failure lists the aliases the keystore actually holds. **And a truncated or mis-pasted base64 is a different problem from a bad password**, so the artifact is described before it is opened: size and its first four bytes, named as PKCS12 or legacy JKS, with a warning when the header is neither. A truncation shows up as 300 bytes against 2564. Verified against real keystores for all five cases: correct, trailing newline, wrong password, wrong alias, truncated base64. Decode and build are one step now. Splitting them would mean either handing the password to a later step through $GITHUB_ENV -- where the env dump is only masked for values that are verbatim a secret, so a trimmed one could print in clear -- or repeating the trimming in both. The failure message also prints the password's length, which is the one thing that distinguishes "wrong value" from "invisible whitespace", and only on failure. |
||
|
|
f7dc76c955 |
docs(android): an arm64 image will not run on an x86_64 host
Build & publish Arch package / arch-package (push) Successful in 2m32s
Search index maintenance / maintain-index (push) Successful in 7s
CI / e2e (push) Successful in 5m42s
CI / check (push) Successful in 2m22s
Sync Homebrew formula / sync-formula (push) Successful in 6s
Build & publish the Android APK / apk (push) Failing after 50s
Emulator 37 refuses cross-architecture emulation outright -- "Avd's CPU Architecture 'arm64' is not supported by the QEMU2 emulator on x86_64 host" -- and there is no flag for it. Google dropped it. That matters because the previous commit's finding points at arm64 as the ABI that works, so the obvious next move is to boot an arm64 AVD, and the obvious next move costs a 3.8 GB download before it fails. Written down so the next session does not spend it. The consequence is stated rather than hidden: the claim that arm64 avoids the seccomp trap rests on reading modernc's two code paths, not on having run it. Verifying it needs an arm64 host, a physical device or adb connect. |
||
|
|
ed975019dc |
fix(dev): the smoke target died silently on a genuinely dead app
Two harness bugs and the finding that exposed them. **`pidof` exits 1 when it finds nothing**, and under `set -e` a failing command substitution killed the script before it could print anything -- rc=1, no output. That was invisible for as long as the app crash-*looped*, because there is always some pid in that state. It appeared the moment the app died for good and ActivityManager stopped respawning it, which is precisely the run you most want output from. **And an install failure said nothing useful.** Both ways it fails are about identity rather than the build: INSTALL_FAILED_VERSION_DOWNGRADE when a bare `make android` (versionCode 1) meets something a versioned build left behind, and a signature mismatch when a debug-signed local build meets a release-signed one. Both were hit in one session, and both are fixed by uninstalling. The target says so now instead of leaving someone to read the constant name. The finding: with the startup bug fixed the app reaches the database and takes SIGSYS on the x86_64 emulator, because modernc.org/libc's Xlstat64 issues a raw lstat syscall on linux/amd64 and Android's seccomp filter forbids it -- bionic never issues it. arm64 has no lstat syscall at all, so ccgo_linux_arm64.go routes Xlstat through fstatat and is structurally unaffected; Go's own syscall package already used fstatat on both. So the default emulator cannot verify this app, and the skill says so rather than letting the next session read a tombstone as a regression. |
||
|
|
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. |
||
|
|
a7a33527c4 |
docs: record what the Android work established and disproved
CLAUDE.md said `wails3 task common:update:build-assets` regenerates build/ios/ and build/android/. It does not: in beta.8 that command extracts only updatable_build_assets, which is darwin/ios/linux/windows, and the android tree comes from `generate build-assets`. It also said nfpm's homepage and license are left alone by the refresh -- a comment in that file says the same -- and a refresh reset them to wails.io and MIT. Both corrected, and the CI section now describes five workflows. NOTES.md gains the measurements: what cross-compiles and what does not, the emulator environment, the Wails Android documentation's own two errors, and the one line that stops the app at runtime -- buildUserDirPath switches on runtime.GOOS and Android takes the default branch returning errUnsupportedOS, so main() calls os.Exit(1) six milliseconds after the JNI bridge comes up. The fix is a documented, build-tag-free API: application.Mobile.StoragePath() returns the app's private files directory and returns "" on desktop, and resolveUserDirPath already lets YJ_HOME override the path on every OS. Deliberately not taken here -- plan 015 is a pipeline, not a port, and the larger question it does not answer is that open-directory dialogs return an error on Android while this app's entire first run is "choose your music folder". |
||
|
|
0c6ca72cf1 |
ci(android): publish a signed APK on every version tag
Builds the fat APK and puts it in Gitea's *generic* package registry, which unlike the repository is readable without credentials -- the reason an Obtainium client can poll a plain URL with no token and no public mirror of the source. A versioned copy for history, a fixed `latest` URL to watch. **Its own workflow, not a job in ci.yml.** That workflow runs on every branch push and is the one that gates; this takes tens of minutes on a cold cache and the runner has capacity 1, so hanging it off the gate would put every push behind an SDK download. **Keyed on the tag.** The ljos pipeline this is modelled on computes a version in CI and cuts the release itself, then gates its Android job on needs.release.outputs.version with an always() whose absence silently kills the manual path. This repo has no release automation -- tags are pushed by hand and homebrew-formula.yml already keys on v* -- so the tag is the version and none of that machinery, or its failure modes, is needed. **No continue-on-error**, which that pipeline does carry: there the Android job shares a workflow with a server deploy that must never go red over a phone build. Here it is standalone and can neither delay nor redden anything, so a release step that fails silently would be strictly worse than one that fails visibly. Four gates before anything is published, each checked against a real APK: a non-empty artifact, both ABIs present, a versionCode equal to the one derived from the tag, and -- verified by pointing it at a deliberately debug-signed build, which it refused -- **not signed with the debug key**. Android refuses to update an app whose signing certificate changed and the only remedy is an uninstall that takes the user's library with it, so the job also refuses to *build* without the keystore secret rather than falling through to Gradle's debug default. The keystore is opened with `keytool -list` before Gradle runs, because Gradle only notices a bad password at :app:validateSigningRelease, a minute of build time in, and reports it as a missing file. And nothing pipes into `head`: under pipefail it exits after one line, the producer takes SIGPIPE and the step fails with 141 having already printed a perfectly good APK. Two secrets, not four. keytool has produced PKCS12 by default since JDK 9 regardless of the .jks extension, and PKCS12 cannot hold a key password distinct from the store password -- given one it says so and ignores it. So ANDROID_KEY_PASSWORD defaults to the store password and the alias to a documented default. The Wails CLI needs no caching hack here: it is a vendored `go tool` and the runner already bind-mounts GOCACHE for every job, so it is warm from ci.yml's own bindings-check. A fourth cache volume for GRADLE_USER_HOME saves ~700MB a run. |
||
|
|
68468e5378 |
feat(dev): an Android failure looks exactly like a success
The APK installs and launches. It also dies six milliseconds later, and finding that out cost a cycle for three reasons that have nothing to do with the bug itself: **Go's stdout does not reach logcat.** An Android app's fd 1 and 2 go to /dev/null, so every slog line -- including the one naming the error the app is about to exit on -- is discarded. `setprop log.redirect-stdio true` does not help: that redirects the Java runtime's System.out, and our code is a c-shared native library. **os.Exit leaves no evidence.** No panic, no AndroidRuntime stack, nothing in /data/tombstones, nothing in `logcat -b crash` or dropbox. All three places anyone would look are empty, and the one signal that is present -- "Zygote: exited due to signal 9" -- reads as "the system killed it" and sends you after the low-memory killer. **ActivityManager restarts it faster than you can observe.** pidof always answers and `am start` always reports Status: ok, so a crash-looping app looks alive. "Did it start" is the wrong question; `make android-smoke` asks whether it is the *same pid* N seconds later, and prints the filtered logcat plus how to read it when it is not. The tell, once known: "I/WailsBridge: Wails bridge initialized" followed immediately by a new pid doing the same thing. scripts/android-emulator.sh follows dev-headless.sh's shape -- background start, saved-PID stop, filtered log tail, never pkill -f. Two scaffold tasks are deliberately not wrapped: `android:logs` greps logcat for (Wails|yellowjacket), which catches the WailsBridge tag but misses the app's own process tag (app.yellowjacket is lowercase) and misses ActivityManager's "has died" line, which is the one that says it crashed; and `ensure-emulator` boots whatever `-list-avds | tail -1` returns, with no pidfile and no boot wait, so it cannot be sequenced. One environment note that is not obvious on Arch: Gradle needs a platform and /opt/android-sdk has none, so ANDROID_SDK defaults to ~/Android/Sdk while ANDROID_NDK points at /opt/android-ndk. Two SDKs, one for each half of the build. |
||
|
|
6fbb62730d |
fix(android): build a release APK that is releasable
Three edits to the scaffold, each of which the generated tree gets
wrong for a shipped app.
**The phone ABI got a debug library.** Upstream's `build` task forwards
ARCH to compile:go:shared but not PRODUCTION, so the arm64 leg
recomputed BUILD_FLAGS against an unset variable and took the debug
branch -- while amd64, which package:fat calls directly with
PRODUCTION: "true", was correct. A release APK therefore shipped a 40MB
unstripped debug library for the only ABI a release is for, beside a
31MB production one for the emulator. 34MB APK before, 27MB after.
**The APK could be installed once and never updated.** Android orders
releases by versionCode and refuses anything not greater than what is
installed; the scaffold hardcodes 1, so the first install would have
been the last and the only way out is an uninstall, which takes the
user's library with it. It comes from YJ_VERSION_CODE now, which CI
derives from the tag (1.3.1 -> 10301, monotonic while minor and patch
stay under 100), with a default that keeps a local build working.
Integer.parseInt, not `(...) as Integer`: Groovy binds the call
parentheses to versionCode before the cast, so the latter reads as
`versionCode("1") as Integer` -- it sets a String, then casts the
setter's null return, and Gradle fails the whole project with "Value is
null" pointing at that line.
**And it identified itself as com.wails.app.** applicationId is
app.yellowjacket now, matching build/config.yml's productIdentifier,
and the label is YellowJacket rather than "Wails App".
Two things follow from that rename and both bite:
The identity is declared twice. applicationId is what Gradle installs;
APP_ID in build/android/Taskfile.yml is what every adb-driven task
uninstalls, launches and filters, and nothing enforces agreement.
ANDROID.md says to set APP_ID in build/config.yml -- that does nothing
in beta.8, checked both ways: `wails3 task` builds its var set from CLI
KEY=VALUE arguments and the Taskfile tree and never reads config.yml,
and even when set it feeds only those adb commands, never Gradle.
And `namespace` deliberately stays com.wails.app, because that is the
Java package MainActivity and WailsBridge live in and renaming it means
renaming their source. So the launcher activity is
app.yellowjacket/com.wails.app.MainActivity, and the short
`.MainActivity` form resolves the dot against the applicationId and
fails with a class-not-found that reads like a broken build.
|
||
|
|
48b37f6301 |
build(android): carry the Wails Android scaffolding verbatim
Plan 015 phase 0 established that this app cross-compiles for Android with no source changes at all. A CGO_ENABLED=0 probe of the whole tree for android/arm64 fails on exactly two packages -- ebitengine/oto/v3 and wails/v3/pkg/application -- and both fail only because their Android implementation is cgo, which is what the NDK supplies. Notably modernc.org/sqlite, the entire database layer and the thing most likely to have no Android target, is clean. The fat APK (arm64-v8a + x86_64) builds in about 25 seconds. So build/android/ stops being ignored. This commit is the tree exactly as `wails3 generate build-assets` emits it, so that the next commit is a readable diff of what we changed and a future refresh has something to compare against. Two things about how it is carried: `wails3 update build-assets` does NOT generate it, contrary to what CLAUDE.md has claimed since the v3 migration. In beta.8 that command extracts only internal/commands/updatable_build_assets, which is darwin/ios/linux/windows; the android tree comes from `generate build-assets`, which rewrites the whole of build/. It was generated once into a scratch directory and copied across, so from here it is committed and hand-edited like source. Only its output is ignored -- jniLibs (~60MB of per-ABI c-shared libraries), gen/, overlay.json and Gradle's own directories. And it brings one Go file into ./... -- scripts/deps/install_deps.go, the interactive SDK installer behind `task android:install:deps`, which trips 24 of our strict linters. golangci excludes the directory rather than reformatting upstream's file, which the next refresh would undo and which would make the diff against upstream unreadable. This repo uses `make android-setup` instead. |
||
|
|
66182f82cd |
fix(indexbuild): repair the one database a squash cannot reach
The index job's /cache volume is a real YJ_HOME that outlives every run, so plan 013's reshaped audio_files met a database still in the old shape: `CREATE INDEX ... album_id` against a table without that column, on every launch. "Delete and rescan" is the squash's answer and is free everywhere except here, where half the file is the catalog and deleting it costs ~205GB of downloading. indexbuild now drops every table datamap does not classify as Cache before the schema is applied. Nothing scans, plays or authors in that database, so its non-catalog half is empty by construction and a shape the schema stopped describing is pure liability; the catalog is never touched. TestRetireLibraryTables reproduces the failure symptom-first: build the real schema, put audio_files back the way the volume had it, assert the open fails, then assert the repair makes it open with the catalog row still there. |
||
|
|
18aba34c08 |
test(e2e): a track plays the list it is in, not a queue of one
|
||
|
|
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. |
||
|
|
c94c97f604 | docs: move plan 009 to completed | ||
|
|
4801ba4480 |
docs: close plan 009, and what a decision phase found
Two of Phase 2's three judgement calls were answered by reading the code rather than by choosing: there is no artist badge to make a button, and a track badge stops reading as noise the moment it means something. The third went the other way — `EntityRecording` reads like a placeholder and is real work. |
||
|
|
40bc968cf8 |
test(e2e): a real click on the badge acts without opening the card
Only this tier can say it: the badge sits inside a card whose own click navigates, so what matters is that a real gesture files the request *and* leaves the page where it was. It clicks a locator rather than a measured point. The first version read a bounding box the moment the search settled, but cover art is still arriving then and a card that grows moves the badge — so the click landed on the card and opened the album, which is precisely the regression the test exists to catch, reported as a failure to file a request. The phase 1 label assertion moves with the component: a control is named after what activating it does, so the badge that said "is queued for download" now says "Cancel the request for …". |
||
|
|
e61b7456df |
feat(explore): make the library badge request what it is on
007 turned this badge from a `<button>` whose handler was a `stopPropagation()` and a TODO into `role="img"`, on the rule that a control which cannot act is worse than none — and wrote down what would change the answer: a `<button>` again *with* a handler, never a handler bolted onto something already shaped like one. This is that. A call site opts in by passing `request-mbid`, so where a badge is redundant it stays a badge: `explore-album-details`'s header has "Want this" in words directly below it, and its template says so by not opting in. An `in-library` badge is never a button either, because there is nothing left to ask for — that is what keeps the tab stops 007 gave back from being spent on nothing. The copy is the action, not the state, and it is deliberately about the request list rather than the library: "Want album X" / "Cancel the request for album X". Clicking still adds nothing to the library, which is what made the original "Add … to library" a promise the control could not keep. Tracks are requestable too. `EntityRecording` is not a placeholder in the request model — `Reconciler.tracklistFor` has a deliberate branch for it, because one expected title is what lets filename matching score a single-track download at all. Artists are not: there is no artist badge anywhere, and a discography subscription belongs on the Follow button that can say what it commits to. The click is swallowed again, for the opposite reason to before: with an action of its own, a click on the badge no longer means what the card means. Enter and Space are stopped for the same reason — every card holding one is a role=button or role=option with its own handler. |
||
|
|
979c6e83ed |
docs: open plan 009 and record what phase 1 found
The plan's own framing was wrong in a way worth keeping: the badge was not waiting on the download client, which had largely landed already — it was waiting on somebody looking at a state nothing produced. |
||
|
|
48f7795687 |
test(e2e): pin the requested badge and the state it renders in
Two assertions, and the second is why this is at this tier at all. Reaching the requested state is the only way to render the requested icon, so the sweep that already asserts `__yjIconMisses` is empty can finally see a name computed from state. Both were watched failing on the pre-fix build by neutering one line each: the badge reported `not-in-library` where `queued` was expected, and the sweep returned `["bookmark-check"]`. The spec gives back what it spends — the request is dropped in `afterAll`, and cleared in `beforeAll` too, since a run that dies between the two would otherwise fail the next one. That cleanup uses the raw binding rather than `callBinding`: a bare `browser.newPage()` has no init script, so the event bridge is undefined and the first version threw where nobody was looking. Its 60 s search budget is not paranoia either. A freshly launched app spends ~40 s merging the core catalog artifact and Explore's search returns nothing until it lands, including for rows staged directly into `explore_index`. |
||
|
|
c400f681c2 |
fix(icons): the "Wanted" button asked for a Pro icon
`bookmark-check` is Font Awesome **Pro**, so it was never bundled and `window.__yjIconMisses` has held it for as long as anything could be requested — the button rendered the missing-icon fallback in the one state it exists to show. `offline-icons.spec.ts` asserts that array is empty and passed anyway: no spec had ever put the app in a state where an album is requested. A name computed from state is only checkable from that state, which is the case `names.txt` exists for. Outline and solid of the same Free glyph carry the toggle instead, which is what the vendoring script tells you to do when a name is missing: pick one that is Free, never reach for the Pro file. |
||
|
|
451b46e63c |
fix(explore): show a requested album as queued, not absent
`library-status-indicator` has had three states since it was written and produced two: all eight call sites were a two-way ternary between `in-library` and `not-in-library`, so the `queued` state it styles and labels was unreachable. The result was the app contradicting itself on one page. An album added to the request list showed a plus and announced "is not in your library", forty pixels from a filled button reading "Wanted". The rule was written at eight places, which is why none of them had all of it, so it is `utils/library-status.ts` now: owning outranks wanting, a satisfied request is not queued, and a request is by MBID — a track inside a requested album is not itself requested and still says so. `explore-view` gains the `downloadStore` subscription both detail views already had, registered `whileActive` because it is a cached view that never unmounts. `top-results-row` needs its own: its host re-rendering sets the same `results` array back, so Lit stops at the property and the row never hears about a change. |
||
|
|
d33dfb2264 |
docs: record phase 4, and the counts a new guard has to agree with
Plan 008 is complete and moves to completed/. The two findings worth carrying forward are that a new table needs one schema file rather than two (and a datamap entry, which is a gate nobody remembers), and that excluding a path has to reach every place that counts what is in the library — the soft scan's disk-vs-database comparison above all, which would otherwise have rescanned the whole library on every launch with nothing failing anywhere. |
||
|
|
41a4dd7148 |
feat(shortcuts): bind tracklist.delete to the confirmation
The binding has been in the defaults and in Settings since it was written, with nothing on the other end of it, because "remove from library" did not exist. It does now — and Delete only *opens* the dialog, never performs the removal, which is the only version defensible one keystroke from a focused row. The e2e case asserts the two things that matter and neither is the row count: the file is still on disk, and a real scan of the real directory does not bring the row back. It watches a control path survive the same scan, because a guard that excluded everything would pass the negative assertion for free — and it restores the database it spends. |
||
|
|
6d97e3c872 |
feat(tracks): remove from library behind a confirmation
The context menu's one destructive command. Its impact line says the files are not deleted, because a user who reads "remove" as "delete" and finds their music gone was failed by the copy rather than by the operation. The store patches rather than invalidates: the event carries the paths, so the tracks array — the expensive collection — is spliced in place and only the album/artist/genre summaries, whose counts really did change, are refetched. It falls back to a full invalidate when a tracks fetch is already in flight, which is the one case a patch cannot be shown to be equivalent to. Deleting an audio_files row cascades to queue_tracks, so the removal also compacts the queue — the same reload RemoveLibrary does, which unloads the player if the removed track was the one playing. |