Move Open Library lookup to the Editions API; stop asking Google Books for its placeholder

Two user-reported shelf-testing symptoms, both a third party answering
misleadingly and the app believing it.

1. Open Library's /api/books?bibkeys=... now 404s for EVERY ISBN, including
   books OL demonstrably still holds, with OL's own x-ol-stats header on the
   response while /isbn/, /search.json, /api/volumes/brief and covers.* all
   serve normally. OL's docs call it the "Legacy Books API" that "may be phased
   out" and it is gone from their API index, so this reads as a retirement
   rather than an outage. The app had been effectively single-sourced on Google
   Books since it broke: every failure the user saw was a book GB lacks.

   Lookup now uses /isbn/{isbn}.json — current, non-legacy, edition-level, and
   the only option of the three that carries a description. /api/volumes/brief
   is a near drop-in for the old response shape and was rejected precisely
   because it is also legacy.

   Its costs, all handled: authors are references, so AuthorNameCache resolves
   and caches them for the process lifetime (books by one author get scanned in
   runs off one shelf); an edition may carry NO authors, in which case they live
   on the work — 9780898707168 on the user's own shelf is exactly this, so
   without the work fallback the move would have silently dropped its author;
   author/work requests are best-effort and can only degrade a record, never
   turn Found into Unavailable.

   404 on this endpoint is authoritative NotFound. The legacy endpoint reported
   a miss as 200 with an empty object, which is why every non-2xx there was a
   failure. Every other non-2xx still is.

2. Google Books answers zoom=2 with a grey "image not available" PNG at HTTP
   200 — not a 404 — for any volume it holds no full preview of. Coil loads it
   as a success, so BookCover's placeholder never fires and the cover pipeline
   uploads Google's placeholder to PocketBase as the book's cover. Measured over
   18 real volumes: 11 placeholders at zoom=2, 0 at zoom=1&w=400. zoom=0/3/6 are
   placeholders too. normalizeCoverUrl now pins zoom=1, adds w=400 and strips
   edge=curl.

SPEC.md's "Book metadata lookup" is rewritten with both rules and the evidence
for them — it was the source of the zoom=2 instruction, and would otherwise be
the reason someone restores it.

Also fixed, because it blocked verification: LibraryViewModelTest never cleared
the view models it built, and LibraryViewModel's eleven WhileSubscribed(5_000)
flows kept running five seconds into later tests, racing resetMain(). It now
cancels each viewModelScope in tearDown.

NOT fixed, reported instead: AddBookViewModel.performSave's in-flight guard is a
check-then-act and two coroutines can both pass it. Unrelated to this change
(that VM has no metadata dependency) and out of scope. See HAZARD #13.

Verified: assembleDebug exit 0; testDebugUnitTest --rerun-tasks 378 tests,
2 skipped, 0 failures (was 355); verifyPaparazziDebug exit 0, no pixels moved;
0 "always 'false'" warnings; no build files touched. LIVE_METADATA=1 live test
ran (not skipped): 9/9 Found with cover art, with the GB key absent, so Open
Library alone answered through the new endpoint.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Spriteandclaude committed 2026-09-20 02:13:01 +00:00
1 parent 4695731f91
commit ab83287b5d
25 files changed
+972 -193

No files matched your search

+63
View File
@@ -896,3 +896,66 @@ code: compare against the gate logs' "BUILD SUCCESSFUL in" times and ask the use
`tasks/wave-chain.sh` now reads its commit subject from `tasks/<task>.subject`
(it was hard-coded for waves 9/10).
## Wave 12 — Open Library endpoint move + Google Books cover fix: DONE 2026-09-20
Done **by the orchestrator directly, not a worker** — the user left the choice open
("do whatever you think will yield the best results"). The change is confined to
`data/metadata`, and the live API shapes had already been measured during diagnosis,
so a worker would have spent a session rediscovering them. Full write-up in
`docs/METADATA-SOURCES.md` § "The Open Library endpoint move".
**Two user-reported symptoms, both third parties answering misleadingly:**
1. `/api/books?bibkeys=...` now 404s for EVERY ISBN, including books OL still holds.
OL's docs call it the "Legacy Books API" that "may be phased out"; it is gone from
their API index. Treat as retired, not down. The app had been effectively
single-sourced on Google Books since it broke.
2. Google Books answers `zoom=2` with a grey "image not available" PNG at **HTTP 200**
for volumes with no full preview — 11 of 18 sampled. SPEC told us to force zoom=2.
**Changed:** `OpenLibraryClient` now uses `/isbn/{isbn}.json` (+ `/works/` fallback
for authors, + `/authors/` resolution via the new `AuthorNameCache`);
`normalizeCoverUrl` pins `zoom=1`, adds `w=400`, strips `edge=curl`. SPEC.md's
"Book metadata lookup" section rewritten with both rules and WHY, so nobody restores
either. Old-endpoint fixtures deleted; 9 new fixtures captured from the live API.
**Three things that would have been silent bugs** (all caught pre-ship, all pinned by
tests): an edition record can carry NO authors (9780898707168 — the user's own shelf —
has them only on the work, so a naive move drops the author); `covers` uses `-1` as a
no-cover sentinel; `description` is a bare string on some records and `{type,value}`
on others. Also: **on this endpoint 404 IS authoritative NotFound**, where on the old
one every non-2xx was a failure.
| Check | Result |
|---|---|
| `./tasks/gw assembleDebug` | exit 0 |
| `./tasks/gw testDebugUnitTest --rerun-tasks` | exit 0 — **378 tests**, 2 skipped, 0 failures (was 355) |
| `./tasks/gw verifyPaparazziDebug` | exit 0 — no pixels moved |
| `LIVE_METADATA=1` live test | **RAN, not skipped** — 9/9 Found with cover art, keyless GB, so OL alone answered |
| `grep "always 'false'"` | 0 hits |
### HAZARD #13 — two latent test races, surfaced by adding tests
Adding ~20 tests shifted suite timing and made two pre-existing races start flapping.
Neither was caused by the metadata change; both flake in whichever test happens to be
running when a window expires, NOT in the one at fault. Do not chase the named test.
1. **FIXED. `LibraryViewModelTest` leaked view models.** `LibraryViewModel` has eleven
`stateIn(viewModelScope, WhileSubscribed(5_000), ...)` flows, so its upstreams keep
running five seconds after the last collector. Nothing ever cleared the VM, so they
were still touching `Dispatchers.Main` while the NEXT test's tearDown called
`resetMain()` -> "Dispatchers.Main is used concurrently with setting it". The test
now tracks every VM it builds and cancels `viewModelScope` in tearDown (and guards
`db.close()` with `::db.isInitialized`, since a failed setUp otherwise masks the
real failure with an UninitializedPropertyAccessException).
2. **NOT FIXED — reported to the user, out of scope.** `AddBookViewModel.performSave`
has a check-then-act in-flight guard: `if (_formState.value.isSaving) return null`
and only then `update { isSaving = true }`. Two coroutines can both pass the check,
which is what `a second save while one is in flight does not create a second book`
catches when timing allows. The KDoc right above it claims a double-tap "still
can't create two books" — that claim is false today. `AddBookViewModel` takes no
metadata dependency at all, so this is provably unrelated to wave 12. One atomic
`getAndUpdate` fixes it.
**Lesson:** a green suite on this project is worth one re-run before you trust it, and
`--rerun-tasks` is mandatory — a plain `testDebugUnitTest` after a stash happily
reports BUILD SUCCESSFUL `FROM-CACHE` without executing a single test. That nearly
produced a false "pre-existing, not mine" conclusion here.