Explore: two things that have not worked since plan 013, and the temp directory Android never had

Conversation 0 Commits 2 Files Changed 7

+485 -34

2 Commits

Author SHA1 Message Date

Author	SHA1	Message	Date
logan	30c6b665f1	fix(system): give the process a temp directory that exists CI / check (push) Skipped CI / e2e (push) Skipped CI / check (pull_request) Successful in 3m0s Details CI / e2e (pull_request) Successful in 9m37s Details Android has no /tmp and hands an app no TMPDIR. Go's os.TempDir() falls back to "/tmp" when the variable is unset, so every library in this process that wants scratch space was being handed a path that has never existed. SQLite is the one that noticed, and it said so precisely: W/yellowjacket: msg="champion index rebuild failed" explore.search-index.error="populate champion fts: disk I/O error (6410)" 6410 is not a generic I/O error. `6410 & 0xff` is 10, SQLITE_IOERR, and `6410 >> 8` is 25 -- SQLITE_IOERR_GETTEMPPATH. SQLite could not work out where to put a temporary file. Two measurements on the device say why: `ls -d /tmp` does not exist, and the app process's environment carries no TMPDIR. A shell's does (/data/local/tmp), which is why this is easy to miss from `adb shell`. The cost was a silent performance cliff on the slowest device this app runs on: `championReady` stayed false, so every Explore search took the generic path over the whole 1,079,667-row index instead of the champion subset, and the rebuild was re-attempted on every launch. The class is fixed rather than the statement. The trigger is the size of the work, not that query -- anything that spills fails the same way there, so large sorts, large joins and VACUUM were all waiting their turn. The repair belongs at the process's one answer to "where do temporary files go". `PRAGMA temp_store = MEMORY` was the alternative: cheaper, more local, and a promise that every future spill fits in RAM on a phone. The catalog is the largest thing in this app and that is not a promise worth making silently. UseTempDir sits beside UseHomeOverride and carries its two rules for the same reasons. An empty base is a no-op, because that is what application.Mobile.StoragePath() returns on desktop -- so this needs no build tag and changes nothing off mobile, where /tmp is real. And an explicit TMPDIR wins, so anyone who set one deliberately gets it; nothing sets it on the platform this exists for. It needs no new Wails API and no Java change: StoragePath() is already what YJ_HOME is pointed at, and the directory goes under it. Two things beyond the rename of a variable. Writability is probed, not assumed. MkdirAll on an existing unwritable directory succeeds, so without the probe this could set TMPDIR to a directory nothing can use -- which is the same bug one directory over, and just as quiet. It returns its error, and main logs it. A temp directory that could not be created is the same silent failure one step earlier. A failure is not fatal: it leaves the platform's answer in place, which is what every release before this one ran with. That log line is readable on the platform only because of #160. Verified on the reference device, where the same launch that used to print the failure now prints: I/yellowjacket: msg="champion index rebuilt" explore.search-index.elapsed=6.496s Closes #190	2026-08-21 17:23:23 -04:00
logan	d034d6e571	fix(explore): resolve pending release MBIDs against the real table The release-group MBID backfill queried `release_groups`, which plan 013 renamed to `albums`. It failed on its first statement on every launch since `e7748f1` and the pass returned quietly having done nothing: W/yellowjacket: msg="release-group mbid backfill: query failed" explore.error="SQL logic error: no such table: release_groups (1)" What it does is resolve a release-level MBID (MUSICBRAINZ_ALBUMID, which many taggers write instead of MUSICBRAINZ_RELEASEGROUPID) into the release-group MBID everything else on the album page is keyed by. A scan cannot afford a live MusicBrainz call, so `library.updateMBIDs` stashes the release MBID in `pending_release_mbid` and defers to this. With this broken the marker was written by every scan and resolved by nothing, so those albums were untagged as far as the catalog is concerned, permanently. The fix is to call the queries plan 013 already wrote. `GetAlbumsWithPendingReleaseMBID` and `ResolveAlbumPendingReleaseMBID` have been in sql/queries/albums.sql since that change, generated and never called -- the writer of the marker was repointed at `albums` and the reader was not. So this is not a missed rename so much as a call site left behind, and thirty lines of raw SQL and hand-rolled scanning become three. That is also the durable half. These two were the last raw-SQL references to a schema table in the tree, and being raw is exactly why 013 missed them: sqlc reads sql/schemas/ and cannot generate against a table that is not declared, which is what made every other statement in the repo immune to the same rename. Three smaller things. The LIMIT came back. The raw statement bounded a run at releaseGroupMBIDBackfillMaxPerRun and 013's sqlc replacement had no LIMIT at all, so switching over as-written would have swapped a dead pass for an unbounded one -- each row is a live MusicBrainz lookup on a 1 req/s limiter shared with every page the user can open. The UPDATE goes through the writer. `ReadQueries` is a query-only pool and an UPDATE issued on it fails at runtime with "attempt to write a readonly database". The query is its own method so its failure is assertable. A test of the pass as a whole cannot see this bug, because a query error and an empty library are the same early return -- which is the whole reason it survived. `pendingReleaseMBIDs` returns the error, and the test reproduces the device's exact message against the old statement. Verified on the reference device: the warning is gone from logcat. Closes #189	2026-08-21 17:23:05 -04:00

logan

30c6b665f1

fix(system): give the process a temp directory that exists

CI / check (push) Skipped

CI / e2e (push) Skipped

CI / check (pull_request) Successful in 3m0s

Details

CI / e2e (pull_request) Successful in 9m37s

Details

Android has no /tmp and hands an app no TMPDIR. Go's os.TempDir() falls
back to "/tmp" when the variable is unset, so every library in this
process that wants scratch space was being handed a path that has never
existed.

SQLite is the one that noticed, and it said so precisely:

    W/yellowjacket: msg="champion index rebuild failed"
      explore.search-index.error="populate champion fts: disk I/O error (6410)"

6410 is not a generic I/O error. `6410 & 0xff` is 10, SQLITE_IOERR, and
`6410 >> 8` is 25 -- SQLITE_IOERR_GETTEMPPATH. SQLite could not work out
where to put a temporary file. Two measurements on the device say why:
`ls -d /tmp` does not exist, and the app process's environment carries
no TMPDIR. A shell's does (/data/local/tmp), which is why this is easy
to miss from `adb shell`.

The cost was a silent performance cliff on the slowest device this app
runs on: `championReady` stayed false, so every Explore search took the
generic path over the whole 1,079,667-row index instead of the champion
subset, and the rebuild was re-attempted on every launch.

**The class is fixed rather than the statement.** The trigger is the
*size* of the work, not that query -- anything that spills fails the
same way there, so large sorts, large joins and VACUUM were all waiting
their turn. The repair belongs at the process's one answer to "where do
temporary files go".

`PRAGMA temp_store = MEMORY` was the alternative: cheaper, more local,
and a promise that every future spill fits in RAM on a phone. The
catalog is the largest thing in this app and that is not a promise
worth making silently.

UseTempDir sits beside UseHomeOverride and carries its two rules for
the same reasons. **An empty base is a no-op**, because that is what
application.Mobile.StoragePath() returns on desktop -- so this needs no
build tag and changes nothing off mobile, where /tmp is real. And **an
explicit TMPDIR wins**, so anyone who set one deliberately gets it;
nothing sets it on the platform this exists for. It needs no new Wails
API and no Java change: StoragePath() is already what YJ_HOME is
pointed at, and the directory goes under it.

Two things beyond the rename of a variable.

**Writability is probed, not assumed.** MkdirAll on an existing
unwritable directory succeeds, so without the probe this could set
TMPDIR to a directory nothing can use -- which is the same bug one
directory over, and just as quiet.

**It returns its error, and main logs it.** A temp directory that could
not be created is the same silent failure one step earlier. A failure
is not fatal: it leaves the platform's answer in place, which is what
every release before this one ran with. That log line is readable on
the platform only because of #160.

Verified on the reference device, where the same launch that used to
print the failure now prints:

    I/yellowjacket: msg="champion index rebuilt"
      explore.search-index.elapsed=6.496s

Closes #190

2026-08-21 17:23:23 -04:00

logan

d034d6e571

fix(explore): resolve pending release MBIDs against the real table

The release-group MBID backfill queried `release_groups`, which plan 013
renamed to `albums`. It failed on its first statement on every launch
since e7748f1 and the pass returned quietly having done nothing:

    W/yellowjacket: msg="release-group mbid backfill: query failed"
      explore.error="SQL logic error: no such table: release_groups (1)"

What it does is resolve a release-level MBID (MUSICBRAINZ_ALBUMID, which
many taggers write instead of MUSICBRAINZ_RELEASEGROUPID) into the
release-group MBID everything else on the album page is keyed by. A scan
cannot afford a live MusicBrainz call, so `library.updateMBIDs` stashes
the release MBID in `pending_release_mbid` and defers to this. With this
broken the marker was written by every scan and resolved by nothing, so
those albums were untagged as far as the catalog is concerned,
permanently.

**The fix is to call the queries plan 013 already wrote.**
`GetAlbumsWithPendingReleaseMBID` and `ResolveAlbumPendingReleaseMBID`
have been in sql/queries/albums.sql since that change, generated and
never called -- the writer of the marker was repointed at `albums` and
the reader was not. So this is not a missed rename so much as a call
site left behind, and thirty lines of raw SQL and hand-rolled scanning
become three.

That is also the durable half. These two were the last raw-SQL
references to a schema table in the tree, and being raw is exactly why
013 missed them: sqlc reads sql/schemas/ and cannot generate against a
table that is not declared, which is what made every other statement in
the repo immune to the same rename.

Three smaller things.

**The LIMIT came back.** The raw statement bounded a run at
releaseGroupMBIDBackfillMaxPerRun and 013's sqlc replacement had no
LIMIT at all, so switching over as-written would have swapped a dead
pass for an unbounded one -- each row is a live MusicBrainz lookup on a
1 req/s limiter shared with every page the user can open.

**The UPDATE goes through the writer.** `ReadQueries` is a query-only
pool and an UPDATE issued on it fails at runtime with "attempt to write
a readonly database".

**The query is its own method so its failure is assertable.**
A test of the pass as a whole cannot see this bug, because a query
error and an empty library are the same early return -- which is the
whole reason it survived. `pendingReleaseMBIDs` returns the error, and
the test reproduces the device's exact message against the old
statement.

Verified on the reference device: the warning is gone from logcat.

Closes #189

2026-08-21 17:23:05 -04:00

Explore: two things that have not worked since plan 013, and the temp directory Android never had #192