Two bugs that #160 surfaced in its first minute of logcat. Both had
been written on every launch and discarded; neither is new, and #189 is
not Android-specific at all — it was simply unread.
commit
issue
fix(explore): resolve pending release MBIDs against the real table
#189 — a call site left behind, not a missed rename
The backfill queried release_groups, which plan 013 renamed to albums, so it failed on its first statement on every launch since e7748f1 and returned quietly having done nothing.
The interesting part is what fixing it turned up: the sqlc queries it
should have been calling have existed since 013. GetAlbumsWithPendingReleaseMBID and ResolveAlbumPendingReleaseMBID
were written by that change, generated, and never called — the writer
of the pending_release_mbid marker was repointed at albums and the reader was not. So the marker was written by every scan and resolved
by nothing, and those albums are untagged as far as the catalog is
concerned, permanently.
Calling them turns thirty lines of raw SQL and hand-rolled scanning into
three, and is the durable half: these 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 immune to the
same rename.
Three things worth reviewing rather than skimming:
The LIMIT came back. 013's sqlc query has none, and the raw
statement it replaces bounded a run at releaseGroupMBIDBackfillMaxPerRun. Switching over as-written would
have traded a dead pass for an unbounded one, and each row is a live
MusicBrainz lookup on a 1 req/s limiter shared with every page the
user can open. Both queries were dead, so adding the parameter breaks
no caller.
The UPDATE goes through Queries, not ReadQueries — the
read pool would refuse it at runtime.
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 precisely why
it survived. The test reproduces the device's exact no such table: release_groups against the old statement.
disk I/O error (6410) decodes: & 0xff is SQLITE_IOERR, >> 8 is
25, which is SQLITE_IOERR_GETTEMPPATH. The device has no /tmp and
the app process has no TMPDIR, so os.TempDir() was handing every
library in the process a path that has never existed. A shell does
have one (/data/local/tmp), which is why this is easy to miss from adb shell.
The trigger is the size of the work rather than that query, so
anything that spills fails identically — large sorts, large joins, VACUUM. The repair is at the process's one answer to the question. PRAGMA temp_store = MEMORY was the alternative and was declined: it is
a promise that every future spill fits in RAM on a phone, and the
catalog is the largest thing in this app.
It needs no new Wails API, which is a correction to what the issue
says. StoragePath() is already what YJ_HOME points at, so the
directory goes under it — no Java change, nothing new from the runtime. UseTempDir sits beside UseHomeOverride and carries its two rules for
the same reasons, so it needs no build tag either. Writability is probed rather than assumed, because MkdirAll on an existing
unwritable directory succeeds and that would be the same silent bug one
directory over.
Verification
make lint (3 configs), make test (3 configs, 36 packages), make ui-test (1007), make e2e (236, chromium — webkit is CI's), make css-check, make bindings-check, make skill-check, tsc --noEmit in frontend/ and e2e/. The frontend gates are
formalities: this PR touches no TypeScript.
make generate re-run and the generated diff committed.
Both test files were proved to bite — six reversions, each
producing the failure the test is named for, including reproducing
the device's own error text for #189.
On the reference device (TLP301, Android 14), the launch that
used to print champion index rebuild failed ... 6410 now prints champion index rebuilt elapsed=6.496s, the tmp directory exists
under the app's storage, and no no such table / query failed / rebuild failed line remains in logcat.
Two bugs that #160 surfaced in its first minute of logcat. Both had
been written on every launch and discarded; neither is new, and #189 is
not Android-specific at all — it was simply unread.
| commit | issue |
|---|---|
| `fix(explore): resolve pending release MBIDs against the real table` | #189 |
| `fix(system): give the process a temp directory that exists` | #190 |
## #189 — a call site left behind, not a missed rename
The backfill queried `release_groups`, which plan 013 renamed to
`albums`, so it failed on its first statement on every launch since
`e7748f1` and returned quietly having done nothing.
The interesting part is what fixing it turned up: **the sqlc queries it
should have been calling have existed since 013**.
`GetAlbumsWithPendingReleaseMBID` and `ResolveAlbumPendingReleaseMBID`
were written by that change, generated, and never called — the *writer*
of the `pending_release_mbid` marker was repointed at `albums` and the
*reader* was not. So the marker was written by every scan and resolved
by nothing, and those albums are untagged as far as the catalog is
concerned, permanently.
Calling them turns thirty lines of raw SQL and hand-rolled scanning into
three, and is the durable half: these 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 immune to the
same rename.
Three things worth reviewing rather than skimming:
- **The `LIMIT` came back.** 013's sqlc query has none, and the raw
statement it replaces bounded a run at
`releaseGroupMBIDBackfillMaxPerRun`. Switching over as-written would
have traded a dead pass for an unbounded one, and each row is a live
MusicBrainz lookup on a 1 req/s limiter shared with every page the
user can open. Both queries were dead, so adding the parameter breaks
no caller.
- **The UPDATE goes through `Queries`, not `ReadQueries`** — the
read pool would refuse it at runtime.
- **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 precisely why
it survived. The test reproduces the device's exact
`no such table: release_groups` against the old statement.
## #190 — the class, not the statement
`disk I/O error (6410)` decodes: `& 0xff` is `SQLITE_IOERR`, `>> 8` is
25, which is `SQLITE_IOERR_GETTEMPPATH`. The device has no `/tmp` and
the app process has no `TMPDIR`, so `os.TempDir()` was handing every
library in the process a path that has never existed. A shell *does*
have one (`/data/local/tmp`), which is why this is easy to miss from
`adb shell`.
The trigger is the **size** of the work rather than that query, so
anything that spills fails identically — large sorts, large joins,
`VACUUM`. The repair is at the process's one answer to the question.
`PRAGMA temp_store = MEMORY` was the alternative and was declined: it is
a promise that every future spill fits in RAM on a phone, and the
catalog is the largest thing in this app.
**It needs no new Wails API**, which is a correction to what the issue
says. `StoragePath()` is already what `YJ_HOME` points at, so the
directory goes under it — no Java change, nothing new from the runtime.
`UseTempDir` sits beside `UseHomeOverride` and carries its two rules for
the same reasons, so it needs no build tag either. Writability is
**probed** rather than assumed, because `MkdirAll` on an existing
unwritable directory succeeds and that would be the same silent bug one
directory over.
## Verification
- `make lint` (3 configs), `make test` (3 configs, 36 packages),
`make ui-test` (1007), `make e2e` (236, chromium — webkit is CI's),
`make css-check`, `make bindings-check`, `make skill-check`,
`tsc --noEmit` in `frontend/` and `e2e/`. The frontend gates are
formalities: this PR touches no TypeScript.
- `make generate` re-run and the generated diff committed.
- **Both test files were proved to bite** — six reversions, each
producing the failure the test is named for, including reproducing
the device's own error text for #189.
- **On the reference device** (TLP301, Android 14), the launch that
used to print `champion index rebuild failed ... 6410` now prints
`champion index rebuilt elapsed=6.496s`, the `tmp` directory exists
under the app's storage, and no `no such table` / `query failed` /
`rebuild failed` line remains in logcat.
Closes #189
Closes #190
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
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
logan
merged commit dad852a8a0 into main2026-08-21 21:43:38 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Two bugs that #160 surfaced in its first minute of logcat. Both had
been written on every launch and discarded; neither is new, and #189 is
not Android-specific at all — it was simply unread.
fix(explore): resolve pending release MBIDs against the real tablefix(system): give the process a temp directory that exists#189 — a call site left behind, not a missed rename
The backfill queried
release_groups, which plan 013 renamed toalbums, so it failed on its first statement on every launch sincee7748f1and returned quietly having done nothing.The interesting part is what fixing it turned up: the sqlc queries it
should have been calling have existed since 013.
GetAlbumsWithPendingReleaseMBIDandResolveAlbumPendingReleaseMBIDwere written by that change, generated, and never called — the writer
of the
pending_release_mbidmarker was repointed atalbumsand thereader was not. So the marker was written by every scan and resolved
by nothing, and those albums are untagged as far as the catalog is
concerned, permanently.
Calling them turns thirty lines of raw SQL and hand-rolled scanning into
three, and is the durable half: these 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 thatis not declared, which is what made every other statement immune to the
same rename.
Three things worth reviewing rather than skimming:
LIMITcame back. 013's sqlc query has none, and the rawstatement it replaces bounded a run at
releaseGroupMBIDBackfillMaxPerRun. Switching over as-written wouldhave traded a dead pass for an unbounded one, and each row is a live
MusicBrainz lookup on a 1 req/s limiter shared with every page the
user can open. Both queries were dead, so adding the parameter breaks
no caller.
Queries, notReadQueries— theread pool would refuse it at runtime.
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 precisely why
it survived. The test reproduces the device's exact
no such table: release_groupsagainst the old statement.#190 — the class, not the statement
disk I/O error (6410)decodes:& 0xffisSQLITE_IOERR,>> 8is25, which is
SQLITE_IOERR_GETTEMPPATH. The device has no/tmpandthe app process has no
TMPDIR, soos.TempDir()was handing everylibrary in the process a path that has never existed. A shell does
have one (
/data/local/tmp), which is why this is easy to miss fromadb shell.The trigger is the size of the work rather than that query, so
anything that spills fails identically — large sorts, large joins,
VACUUM. The repair is at the process's one answer to the question.PRAGMA temp_store = MEMORYwas the alternative and was declined: it isa promise that every future spill fits in RAM on a phone, and the
catalog is the largest thing in this app.
It needs no new Wails API, which is a correction to what the issue
says.
StoragePath()is already whatYJ_HOMEpoints at, so thedirectory goes under it — no Java change, nothing new from the runtime.
UseTempDirsits besideUseHomeOverrideand carries its two rules forthe same reasons, so it needs no build tag either. Writability is
probed rather than assumed, because
MkdirAllon an existingunwritable directory succeeds and that would be the same silent bug one
directory over.
Verification
make lint(3 configs),make test(3 configs, 36 packages),make ui-test(1007),make e2e(236, chromium — webkit is CI's),make css-check,make bindings-check,make skill-check,tsc --noEmitinfrontend/ande2e/. The frontend gates areformalities: this PR touches no TypeScript.
make generatere-run and the generated diff committed.producing the failure the test is named for, including reproducing
the device's own error text for #189.
used to print
champion index rebuild failed ... 6410now printschampion index rebuilt elapsed=6.496s, thetmpdirectory existsunder the app's storage, and no
no such table/query failed/rebuild failedline remains in logcat.Closes #189
Closes #190
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