Review fixes for waves 9-10; preload covers so Save stops waiting on them
Orchestrator review ofb600df6andcf82cbc(the chain's gate only proved they built). Fixed: - Search: "In your library" now shows the merged set (owned editions reached only via an online ISBN were removed from "Online" and shown nowhere); the shelf filter applies to it and hidden owned matches are counted. - "Edit details" closes the online save sheet (left open, it invited a duplicate save). - A failed remembered-shelf write after a successful insert no longer reports "Couldn't save" (a retry would duplicate the book). - Library search no longer normalizes every book on every keystroke on the main thread. - Add-by-hand duplicate check and the scan sheet's save are guarded (no crash, no double-tap duplicate, CancellationException rethrown). Covers (design decided with the user): createBook used to download the cover before writing the row, so every save waited on the image host, and a failed download silently saved no cover file. The cover now downloads as soon as a sheet has its URL; Save waits for it if needed and copies the file into place. A failed download shows on the sheet with Retry, and Save becomes "Save without cover". Unsaved preloads are discarded, and leftovers from a killed process are swept at startup. 337 tests (was 308), 2 skipped, 0 failures; assembleDebug and verifyPaparazziDebug green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
cf82cbc484
commit
3c30a20324
29 files changed
+1447
-142
No files matched your search
@@ -766,3 +766,72 @@ Logs: `logs/wave-chain.log`, `logs/gate-<task>.log`, `logs/WAVE9-DONE`, `logs/WA
|
||||
**The gate is not acceptance.** Still owed by the orchestrator for BOTH waves: read each
|
||||
`.summary`, read the diff, eyeball the new Paparazzi PNGs, check CancellationException is
|
||||
rethrown in every new catch, and check wave 10's dedupe judgement call in its report.
|
||||
|
||||
## Waves 9 + 10 — REVIEWED 2026-09-16; accepted after a review-fix round
|
||||
Chain finished 2026-09-15 18:28Z: `b600df6` (wave 9, 241 tests), `cf82cbc` (wave 10, 308).
|
||||
Orchestrator independently re-ran assembleDebug / testDebugUnitTest / verifyPaparazziDebug on
|
||||
both — green — then read the diffs and PNGs. The gate was right that they built; it could not
|
||||
see these, all fixed directly by the orchestrator in the review-fix commit that follows:
|
||||
|
||||
1. **"In your library" never showed the merged set.** The screen passed the plain text match
|
||||
(`uiState.books`) instead of `OnlineSearchState.Done.inLibrary`, so an owned edition reached
|
||||
only via an online ISBN was removed from "Online" and shown NOWHERE; wave 10's VM test
|
||||
asserted `inLibrary` but nothing rendered it. Now rendered; the shelf filter applies to it
|
||||
and hidden owned matches are counted ("1 more in your library, outside the current filter").
|
||||
2. **"Edit details" left the save sheet open** -> save from the add screen, Back, sheet still up
|
||||
with Save enabled -> duplicate book. `LibraryViewModel.takeForEditing()` closes it.
|
||||
3. **`rememberShelf` inside the save `try`** (add + online save): a failed prefs write after a
|
||||
successful insert showed "Couldn't save" -> retry -> duplicate. Now after, and swallowed.
|
||||
4. **Per-keystroke search on the main thread**: two regexes recompiled per call, every book
|
||||
re-normalized per keystroke (~60ms/keystroke at 2,000 books on this server, measured with
|
||||
jshell). Precompiled regexes, `LocalBookMatcher.index` rebuilt only when books change,
|
||||
`flowOn(Dispatchers.Default)`.
|
||||
5. **Add-by-hand duplicate check unguarded** (a Room throw from a keystroke would crash). Guarded.
|
||||
6. **Scan screen save was unguarded** (wave 8's known gap): now catch/rethrow-cancellation,
|
||||
in-flight guard, error line. `performSaveManualEntry` (post-lookup NotFound sheet) is STILL
|
||||
unguarded — its sheet has no error UI; left as is.
|
||||
|
||||
### Cover preload — design decided WITH the user, 2026-09-16
|
||||
User-reported: saving had a distinct delay even with the cover already visible on the sheet.
|
||||
Cause: `createBook` downloaded the cover (fresh OkHttpClient) BEFORE writing the row, and on
|
||||
failure silently saved with no `localCoverPath`. User rejected "save first, fetch cover in the
|
||||
background" (a kill/crash loses the upload; a failure has nowhere to be shown). Their design:
|
||||
- **Preload** (`data/repo/CoverPreload.kt`): the download starts when a sheet/screen gets a
|
||||
cover URL — scan Found sheet, online save sheet on open (ONE result, never the whole list —
|
||||
the user asked), add screen when "Edit details" carries a cover. Into `cacheDir/cover-preload/`.
|
||||
- **Save waits** for it if still running; `createBook(coverFile=)` COPIES it to
|
||||
`filesDir/covers/<id>.jpg` (copy, so a failed insert leaves the preload for a retry; the copy
|
||||
is deleted on insert failure).
|
||||
- **Failure = Option A (user's choice)**: failure line + Retry on the sheet; Save becomes
|
||||
"Save without cover". A download that fails WHILE Save waits stops the save (the user hasn't
|
||||
seen it yet); one that had already failed when tapped proceeds without the file.
|
||||
- **Cleanup**: preload discarded on skip/dismiss/save/onCleared; `BookshelfApplication` sweeps
|
||||
files older than process start. NOT Coil's disk cache — public API, but Coil's eviction and
|
||||
keys aren't a contract; the user agreed it wasn't worth depending on.
|
||||
- Downloads use `metadataHttpClient` (never the PocketBase-token client).
|
||||
|
||||
### Still open after review (not fixed — judgement calls or out of scope)
|
||||
- Wave 9 has NO worker report (worker hit its wall clock mid-verification, see hazard #11). Its
|
||||
undocumented calls: small EditNote FAB above Scan; no "More fields…" on the in-scan sheet;
|
||||
`imePadding` rather than the prompt's `safeDrawingPadding` — needs an on-device look.
|
||||
- Dedupe (wave 10's own analysis, agreed): false merges for same-author colon series
|
||||
("Star Wars: X"/"Star Wars: Y") and same-title same-surname authors; one bad edge chains via
|
||||
union-find; misses translations, surname spelling variants, author-less books.
|
||||
- Retry re-runs BOTH sources (an OL failure spends a Google Books request).
|
||||
- Search toolbar still renders as a blank bar in Paparazzi (pre-existing), so no snapshot shows
|
||||
a query. Online result rows are a non-lazy Column (≤ ~40 thumbnails per search; accepted).
|
||||
|
||||
### HAZARD #11 — the lease did NOT stop a freeze; the wall clock counts frozen time
|
||||
Chain took the lease 2026-09-13 17:30 (expire 3600s). The sprite froze ~17:45 — BEFORE the
|
||||
first renewal, lease still valid — and stayed frozen ~26.5h until the user connected 09-14
|
||||
20:21. No reboot (uptime continuous); processes paused (`ps` start times shifted by the gap).
|
||||
It froze again during K-manual-fix (09-14 20:37 -> 23:43 -> 09-15 11:27 log gaps).
|
||||
Consequences: (a) an unattended chain only progresses while someone is connected — do not
|
||||
promise the user overnight progress; (b) `run-task.sh`'s 24h `wall=` counts the freeze, so
|
||||
K-manual was killed as "WALL CLOCK EXCEEDED" seconds after waking, mid-test-run, with no report.
|
||||
The 8 red tests the gate then saw were its own unfinished tests. If this recurs, measure elapsed
|
||||
time with the monotonic clock (`/proc/uptime`), which does not advance while frozen.
|
||||
|
||||
Also seen 2026-09-16: Claude Code's background-task runner killed Gradle runs "because the system
|
||||
is running low on memory" even with ~6GB available (8GB box, no swap; Kotlin daemon ~2.1GB,
|
||||
Gradle daemon ~1.2GB). Foreground runs of the same command succeeded.
|
||||
Reference in new issue
Block a user