Plan waves 9-10: manual entry, then online search; chained runner
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RWoivrRUEmJLFFwbsrGkqQ
This commit is contained in:
Spriteandclaude committed
1 parent
0c0b8466cc
commit
9b74f50565
4 files changed
+605
No files matched your search
@@ -0,0 +1,162 @@
|
||||
You are implementing ONE feature in the Bookshelf Android app at ~/bookshelf.
|
||||
|
||||
READ FIRST, in this order:
|
||||
1. ~/bookshelf/docs/SPEC.md — the authoritative contract, especially "Non-negotiables",
|
||||
"Design language" and "Screens". Do not contradict it and do not edit it.
|
||||
2. The files named below, and their neighbours, before you write anything.
|
||||
|
||||
## The gap
|
||||
|
||||
There is currently NO way to add a book that has no barcode. Every path into
|
||||
"save a book" starts from an ISBN:
|
||||
- The scan screen's keyboard button (`ui/scan/ScanScreen.kt`, `showManualEntry`)
|
||||
only accepts an ISBN and then runs a metadata lookup on it.
|
||||
- `ManualEntrySheet` (ScanScreen.kt) only appears after a lookup comes back
|
||||
NotFound / LookupFailed, and `ScanViewModel.saveManualEntry(isbn13: String, ...)`
|
||||
REQUIRES an ISBN.
|
||||
Older books, pamphlets, self-published and foreign books often have no ISBN at all.
|
||||
The data layer already supports this: `BookRepository.createBook(isbn13: String? = null, ...)`
|
||||
takes every field as optional except title. This wave is UI + navigation work.
|
||||
|
||||
A follow-up wave (online search, runs right after you) will navigate into what you
|
||||
build here with a pre-filled title, and possibly other pre-filled fields. Design for
|
||||
that; do not build the search.
|
||||
|
||||
## What to build
|
||||
|
||||
### 1. An "Add a book by hand" screen
|
||||
|
||||
New package `org.modg.bookshelf.ui.add` — `AddBookScreen` + `AddBookViewModel`.
|
||||
A full screen (not a sheet: the library has no camera context, and this form is
|
||||
longer than the scan sheet's two fields). Fields, all optional except Title:
|
||||
Title (required), Subtitle, Authors (comma-separated, same convention as
|
||||
ManualEntrySheet), Publisher, Published (free text — SPEC's `publishedDate` is a
|
||||
text field; "1960" and "1999-04" are both legitimate), Pages (digits only),
|
||||
ISBN, Description, and a shelf picker.
|
||||
|
||||
- **Pre-fill.** Model the form as a plain data class (e.g. `BookDraft`) and let the
|
||||
ViewModel take an initial draft. Add a route in `ui/nav/Routes.kt` with OPTIONAL
|
||||
query arguments for at least `title` and `isbn`, e.g. `add?title={title}&isbn={isbn}`
|
||||
plus a helper `Routes.addBook(title: String? = null, isbn: String? = null)` that
|
||||
URL-encodes its arguments. (Titles contain `&`, `?`, `/`, `#` — test that the
|
||||
helper round-trips them.) The next wave may extend the draft; keep the draft →
|
||||
form mapping in one place so that is a small change.
|
||||
- **ISBN is optional but never silently discarded.** Blank = no ISBN. Non-blank must
|
||||
parse via `IsbnUtils.toIsbn13` (accepts ISBN-10 or -13, with or without hyphens);
|
||||
if it does not, mark the field in error with a message and disable Save. Wave 5
|
||||
fixed exactly this silent-discard bug in the manual-ISBN dialog — do not
|
||||
reintroduce it. Store the normalized ISBN-13; also store the ISBN-10 when the user
|
||||
typed a valid ISBN-10.
|
||||
- **Duplicate warning.** If the parsed ISBN-13 is already owned
|
||||
(`BookRepository.findByIsbn13`), show the same kind of warning the scan sheet
|
||||
shows (`DuplicateCheck` / `DuplicateStatus` in `ui/scan/ScanModels.kt` — reuse,
|
||||
do not duplicate). It is a warning, not a block: two copies of a book is legal.
|
||||
- **Shelf picker.** Reuse the existing shelf picker exactly as the scan sheet uses it
|
||||
(`ui/components/ShelfPickerSheet.kt` and the `ShelfPicker` call in ScanScreen.kt),
|
||||
including pre-selecting the remembered last shelf from `SettingsStore`, and
|
||||
remembering the chosen shelf on save the same way `ScanViewModel.rememberShelf` does.
|
||||
- **Two save actions**, because SPEC's real use case is shelving a box of books:
|
||||
"Save" — saves, then navigates to the new book's detail screen, removing the add
|
||||
screen from the back stack (so Back from detail returns to the library).
|
||||
"Save & add another" — saves, clears the form but KEEPS the shelf, shows a short
|
||||
confirmation naming the saved title, and a running count, and puts focus back in
|
||||
Title.
|
||||
- **Saving must not crash or lose the form.** Wave 8 made the lookup path
|
||||
exception-safe but flagged that SAVE paths still make unguarded Room calls. Guard
|
||||
yours: catch `Throwable` around the save, RETHROW `CancellationException` first
|
||||
(`kotlinx.coroutines.CancellationException`), and on failure keep the typed form
|
||||
and show an error naming the exception class (not its message). Disable the save
|
||||
buttons while a save is in flight so a double-tap cannot create two books.
|
||||
- Offline-first (SPEC): Room only. Nothing on this screen touches the network.
|
||||
- IME: Next between fields, sensible keyboard types (Number for pages, capitalize
|
||||
words for title/authors), and make sure the focused field is not hidden behind
|
||||
the keyboard — the setup screen hit this; see how it was fixed there
|
||||
(`safeDrawingPadding` outside `verticalScroll`, `adjustResize` is already set).
|
||||
- **Split a stateless `AddBookContent(state, callbacks)` out of the screen** so the
|
||||
Paparazzi snapshot renders the REAL composable. Wave 6 shipped a hand-rolled
|
||||
lookalike snapshot for Locations and it is recorded in HANDOFF.md as a known
|
||||
soft spot; do not repeat that.
|
||||
|
||||
### 2. Entry points
|
||||
|
||||
- **Library screen** (`ui/library/LibraryScreen.kt`): add an entry point next to the
|
||||
scan FAB. Use a Material 3 pattern that keeps Scan as the primary action — e.g. a
|
||||
`SmallFloatingActionButton` ("Add by hand", edit/pencil-style icon) stacked above
|
||||
the main FAB. Pick what looks right against SPEC's design language; justify it in
|
||||
your report. Also give the empty state a secondary "Add by hand" action next to
|
||||
"Scan a book". New `LibraryScreen` parameter, wired in `BookshelfNavHost`.
|
||||
- **Scan screen**: the manual-ISBN dialog (the keyboard button) gets a clear
|
||||
secondary action like "No ISBN? Enter the details by hand" that navigates to the
|
||||
add screen. New `ScanScreen` callback parameter, wired in `BookshelfNavHost`.
|
||||
- Leave the in-scan `ManualEntrySheet` (post-lookup NotFound/LookupFailed) as it is —
|
||||
it keeps the scanned ISBN and continuous-scan flow. You may give it a
|
||||
"More fields…" action that opens the add screen pre-filled with that ISBN and the
|
||||
title/authors typed so far, if it is small; otherwise leave it and say so.
|
||||
|
||||
## Constraints — these are hard
|
||||
|
||||
- Kotlin, Compose, Material 3. Match surrounding code's style, naming and comment
|
||||
density (this codebase comments the evidence for a decision, not a restatement of
|
||||
the code). Read neighbouring files first.
|
||||
- You MAY touch: new `ui/add/`, `ui/nav/`, `ui/library/`, `ui/scan/` (entry point
|
||||
and optional "More fields…" only), `ui/components/` (only if a shared piece must
|
||||
move there to be reused), and tests. Nothing else.
|
||||
- DO NOT touch: any build file (`app/app/build.gradle.kts`, `gradle/libs.versions.toml`,
|
||||
etc.), `data/**`, `server/`, `docs/`. If you believe you need a dependency or a
|
||||
data-layer change, STOP and say so in your report. Everything above is doable with
|
||||
what exists (`createBook` already takes all these fields).
|
||||
- DO NOT read, print or grep `app/local.properties`.
|
||||
- DO NOT git commit, add or push. Leave the work in the working tree.
|
||||
- Build with `./tasks/gw <task>` — NEVER `./gradlew` directly (flock-serialized wrapper).
|
||||
- RUN BUILDS IN THE FOREGROUND. Never background a Gradle build and end your turn
|
||||
saying you will report later — you will never get to. Several previous workers on
|
||||
this project did exactly that. A build takes 1-5 minutes; wait for it.
|
||||
- No emulator on this box. You cannot run the app. Do not claim you did.
|
||||
|
||||
## Definition of done
|
||||
|
||||
Run these yourself, in the foreground, and paste the real output tails:
|
||||
|
||||
./tasks/gw assembleDebug -> exit 0
|
||||
./tasks/gw testDebugUnitTest -> exit 0. **205 tests today, 2 skipped** (both
|
||||
opt-in live tests). Count must go UP; nothing
|
||||
may regress. Sum the TEST-*.xml files
|
||||
(app/app/build/test-results/testDebugUnitTest/)
|
||||
— do not read the console.
|
||||
./tasks/gw recordPaparazziDebug -> exit 0
|
||||
./tasks/gw verifyPaparazziDebug -> exit 0 against the re-recorded snapshots
|
||||
|
||||
Required tests with REAL assertions (assertion-free tests violate SPEC):
|
||||
- `Routes.addBook` round-trips titles containing `&`, `?`, `/`, `#`, spaces and
|
||||
non-ASCII (e.g. "Hänsel & Gretel / Part 1?"), and omits absent args.
|
||||
- Draft validation: blank title disables save; blank ISBN is valid (saves null);
|
||||
ISBN-10 is normalized to ISBN-13 and the ISBN-10 kept; a bad checksum is an
|
||||
error, not a silent drop; non-digit pages rejected.
|
||||
- ViewModel: save calls `createBook` with every field mapped (authors split and
|
||||
trimmed, empty entries dropped); duplicate ISBN produces the AlreadyOwned
|
||||
warning; "save & add another" clears fields but keeps the shelf and increments
|
||||
the count; a repository that throws leaves the form intact with an error and
|
||||
does not throw; a `CancellationException` is NOT swallowed; a second save while
|
||||
one is in flight does not create a second book.
|
||||
- Paparazzi (light + dark, in `ui/screens/` alongside the others): the empty form,
|
||||
a filled form with a duplicate-ISBN warning, and the ISBN-error state. Plus the
|
||||
library screen with the new FAB, re-recorded.
|
||||
|
||||
Also grep the build output for Kotlin warnings on files you touched. "Check for
|
||||
instance is always 'false'" is NOT cosmetic — it hid a bug that blanked every book
|
||||
cover in this app for months.
|
||||
|
||||
## Report
|
||||
|
||||
End with a plain report:
|
||||
- what you changed, file by file
|
||||
- verbatim tails of each gradle command above
|
||||
- test count before and after, summed from the XML
|
||||
- the exact route string and `Routes.addBook` signature, and how `BookDraft` is
|
||||
passed in — the next wave's worker will read this report to build on it
|
||||
- judgement calls you made (FAB pattern, "More fields…" yes/no) and why
|
||||
- anything you could NOT do or did differently, and why
|
||||
- anything you noticed that looks wrong but was out of scope
|
||||
|
||||
Be honest. Workers here have over-claimed before, and everything is re-verified, so
|
||||
an inflated report only wastes a round trip.
|
||||
@@ -0,0 +1,252 @@
|
||||
You are implementing ONE feature in the Bookshelf Android app at ~/bookshelf.
|
||||
|
||||
READ FIRST, in this order:
|
||||
1. ~/bookshelf/docs/SPEC.md — the authoritative contract, especially "Non-negotiables",
|
||||
"Book metadata lookup" (the three-way outcome rule), "Design language", "Screens".
|
||||
Do not contradict it and do not edit it.
|
||||
2. ~/bookshelf/docs/METADATA-SOURCES.md — "Measured again 2026-09-09" and "The key
|
||||
landed". Open Library is slow and long-tailed (median ~4s, max 22s) and fails
|
||||
~13% of the time with fast TLS resets; Google Books is keyed at 1,000 req/day.
|
||||
3. `logs/K-manual.summary` and `git show --stat HEAD` — the PREVIOUS wave (just
|
||||
committed) added a manual "Add a book by hand" screen (`ui/add/`) with a route
|
||||
that takes a pre-filled title/ISBN and a `BookDraft`. You will build on it.
|
||||
Read its code, not just the report.
|
||||
4. `data/metadata/` in full (it is small), `ui/library/`, `ui/nav/`.
|
||||
|
||||
## The feature
|
||||
|
||||
Today the library search bar filters only the user's own books, as they type. Extend
|
||||
it to also search Open Library and Google Books, so a user can find and add a book by
|
||||
title and/or author — including "book name + author name" when the title is common.
|
||||
|
||||
The user's design, and the decisions they have already made — do not relitigate:
|
||||
1. User types a query into the existing library search bar.
|
||||
2. Own library filters AS THEY TYPE (instant, offline, as today).
|
||||
Online search runs ONLY ON SUBMIT (IME Search action, or an explicit button) —
|
||||
never per keystroke. Decided by the user: per-keystroke would burn the 1,000/day
|
||||
Google Books quota and results would thrash at OL's latency.
|
||||
3. Results show an "In your library" section on top and online results below.
|
||||
4. Results are DEDUPLICATED across the three sources (local, OL, GB): a book appears
|
||||
once. "In your library" means the user owns SOME edition of that book — decided
|
||||
by the user; an online hit that is a different edition of an owned book belongs
|
||||
in the library section, not the online one.
|
||||
5. Tapping an online result opens a save sheet (same idea as the scan screen's Found
|
||||
sheet: book summary + shelf picker + Save / Skip). ISBN is filled in ONLY when it
|
||||
is unambiguous — see "ISBN rule" below.
|
||||
6. At the bottom of the online results: "Can't find it? Enter it by hand" -> the
|
||||
previous wave's add screen, pre-filled with the query as the title.
|
||||
|
||||
## Verified API facts (orchestrator tested these live on 2026-09-13 — trust them)
|
||||
|
||||
Both APIs treat a free-text `q` as matching across title AND author, so
|
||||
"hittite warrior williamson" and "the hobbit tolkien" both return the right book first.
|
||||
|
||||
**Open Library** — `https://openlibrary.org/search.json?q=<q>&limit=20&fields=<list>`
|
||||
- Results are WORKS (`key` = "/works/OL2028038W"), not editions.
|
||||
- **`isbn` is NOT in the default response. You MUST pass `fields=`** or you will get
|
||||
no ISBNs at all. Use:
|
||||
`fields=key,title,subtitle,author_name,first_publish_year,edition_count,cover_i,isbn,publisher,number_of_pages_median`
|
||||
- `isbn` is a flat list mixing ISBN-10 and ISBN-13 of ALL editions, e.g.
|
||||
["1883937388","9781883937386"] (one edition) or dozens for a popular work. It may
|
||||
be absent (pre-ISBN books). `cover_i` may be null.
|
||||
- Cover by id: `https://covers.openlibrary.org/b/id/{cover_i}-M.jpg`. That is a cover
|
||||
the source REPORTED, so it is legitimate under SPEC's cover rule. Never synthesize a
|
||||
by-ISBN cover URL.
|
||||
- Observed latency 3-6.5s. Real example of a near-duplicate: the same book returned as
|
||||
two works, authors "Joanne S. Williamson" and "Joanne Small Williamson".
|
||||
**Google Books** — `https://www.googleapis.com/books/v1/volumes?q=<q>&maxResults=20&printType=books&key=<key>`
|
||||
- Results are EDITIONS (volumes). `volumeInfo.industryIdentifiers` may be null.
|
||||
`imageLinks` may be absent. `publishedDate` can be garbage ("101-01-01" observed).
|
||||
- Popular titles return several editions of the same work ("The Hobbit" x5, author
|
||||
strings "J.R.R. Tolkien", "J. R. R. Tolkien", "John Ronald Reuel Tolkien").
|
||||
- Observed latency ~0.7s. Same key and same redaction concerns as `GoogleBooksClient`.
|
||||
- Returns loosely-related noise after the real hits. Do not try to filter it; keep
|
||||
source order.
|
||||
|
||||
## What to build
|
||||
|
||||
### 1. Search clients (`data/metadata/`)
|
||||
|
||||
A `search(query): SearchSourceResult` per source — add to the existing clients or add
|
||||
sibling classes, your call, but REUSE their machinery rather than copying it:
|
||||
- the same `OkHttpClient`, `withRetry` / `RetryPolicy`, `FailureKind`,
|
||||
`SourceResult.fromException` / `fromHttpCode`, and for GB the key handling,
|
||||
`requestUrl`-style pure URL construction, and `redact`.
|
||||
- three-way per source, never a bare null: Found(list) / NotFound (authoritative
|
||||
empty) / Failed(kind, reason). SPEC: a source that could not be reached must never
|
||||
be presented as "no results".
|
||||
- the wave-8 exception safety, exactly: never throws, `Throwable` -> `UNEXPECTED`
|
||||
with the exception class name only, `CancellationException` RETHROWN FIRST.
|
||||
- URL-encode the query via `HttpUrl.Builder` (not string concatenation).
|
||||
- GB cover URLs through the existing https+zoom normalization in `GoogleBooksDtos.kt`
|
||||
(it is `private` today; make it `internal` rather than duplicating it).
|
||||
- Wire into `AppContainer` (one or two lines; you may edit that file for this).
|
||||
|
||||
### 2. Local matching
|
||||
|
||||
The current `BookDao.search` matches the whole string as one LIKE, so "hobbit tolkien"
|
||||
matches nothing. Replace it for this screen with token matching: normalize the query
|
||||
and the book (lowercase, strip diacritics, treat punctuation as space so "J.R.R." ~
|
||||
"j r r"), split the query on whitespace, and require EVERY token to appear in at least
|
||||
one of title, subtitle, authors, isbn13, isbn10. A home library is hundreds to low
|
||||
thousands of books, so a pure Kotlin matcher over the already-observed book list is
|
||||
fine and far more testable than SQL — prefer that. If you do change `BookDao.search`
|
||||
instead, its Robolectric DAO tests must be updated, not deleted.
|
||||
|
||||
### 3. Merge + dedupe (pure, no Android, heavily tested)
|
||||
|
||||
A pure object (e.g. `SearchResultMerger`) taking the local books and each source's
|
||||
result list, returning something like
|
||||
`SearchResults(inLibrary: List<BookEntity>, online: List<OnlineBook>)`.
|
||||
|
||||
Identity — two records are the same book if EITHER:
|
||||
a) they share any ISBN after normalizing everything to ISBN-13
|
||||
(`IsbnUtils.toIsbn13`) — this is what joins a GB edition to its OL work, or
|
||||
b) normalized title AND first author's surname match. Normalized title: lowercase,
|
||||
strip diacritics and punctuation, drop a leading "the/a/an", drop anything after
|
||||
the first ':' (subtitles). Surname: last alphabetic token of the first author.
|
||||
("Joanne S. Williamson" ~ "Joanne Small Williamson"; "J.R.R. Tolkien" ~
|
||||
"John Ronald Reuel Tolkien".)
|
||||
Identity is transitive: group with union-find (or equivalent), not pairwise.
|
||||
|
||||
Placement:
|
||||
- Any group containing a local book -> "In your library", shown as the local book(s).
|
||||
This includes local books reached ONLY via an online hit (the online record matched
|
||||
by ISBN/title but the local tokens did not).
|
||||
- Every other group -> ONE `OnlineBook` in the online section. Order by the group's
|
||||
best rank in either source (min position), so relevance is preserved and neither
|
||||
source is buried under the other.
|
||||
- Field fill for a merged OnlineBook: title/authors from OL if present (work-level,
|
||||
cleaner), else GB; year from OL `first_publish_year` else the year GB's
|
||||
`publishedDate` starts with IF it is a plausible 4-digit year; cover from OL
|
||||
`cover_i` else GB thumbnail; remember which sources contributed.
|
||||
|
||||
**ISBN rule** (decided by the user — implement exactly):
|
||||
- If the group has an OL work whose ISBNs normalize to EXACTLY ONE distinct ISBN-13
|
||||
-> use it. (A single-edition work, or one whose only ISBN-bearing edition is that
|
||||
one — ISBN-10 and ISBN-13 of the same edition are ONE ISBN.)
|
||||
- Else if the group's GB volumes carry exactly one distinct ISBN-13 -> use it (GB
|
||||
returned that specific edition for this query).
|
||||
- Otherwise -> no ISBN. Never guess among several.
|
||||
|
||||
### 4. Library screen UI (`ui/library/`)
|
||||
|
||||
- IME action Search on the existing field triggers the online search. Because an IME
|
||||
action is invisible, ALSO show, whenever the query is non-blank and no online search
|
||||
has run for it, a clearly tappable row below the local results:
|
||||
"Search Open Library & Google Books for "<query>"". Minimum query 2 characters.
|
||||
- Sections: "In your library" (existing card style) then "Online" results as rows
|
||||
(small cover, title, authors, year). If there are no local matches, say so in one
|
||||
quiet line rather than hiding the section header — the user must be able to tell
|
||||
"not owned" from "didn't look".
|
||||
- Online states: loading (with the local section fully usable meanwhile — SPEC: no
|
||||
screen may block on network); results; authoritative no-results; per-source failure
|
||||
as a quiet line naming the source and its `reason` (e.g. "Google Books couldn't be
|
||||
reached — tls connection reset"), with Retry, while still showing the OTHER source's
|
||||
results; both failed -> a retry affordance that does NOT say "no results".
|
||||
- Editing the query after a submit clears the online section (and cancels any in-flight
|
||||
search): stale results for a different query are misleading.
|
||||
- Shelf/bookcase filter keeps applying to the local section only.
|
||||
- Tap an online result -> save sheet (bottom sheet on the library screen): cover,
|
||||
title, authors, year, ISBN if the rule gave one, shelf picker (reuse
|
||||
`ui/components/ShelfPickerSheet.kt` / `ShelfPicker` the way the scan sheet does,
|
||||
remembered last shelf included), buttons Save / "Edit details" / Skip.
|
||||
* Save -> `BookRepository.createBook` with everything known, `coverSourceUrl` =
|
||||
the cover URL (the repository already downloads it best-effort). Guard it: catch
|
||||
Throwable, rethrow CancellationException, show an error, keep the sheet. No
|
||||
double-tap double-save. After a save the book now appears in "In your library"
|
||||
by itself (Room flow) — verify that in a test rather than hand-moving it.
|
||||
* If an ISBN is known, ENRICH in the background: call the existing
|
||||
`MetadataRepository.lookup(isbn)` when the sheet opens and fill blanks
|
||||
(publisher, pages, description, better cover) via the same fill-blanks idea as
|
||||
`MetadataMerger` when it lands. Save must NEVER wait for this — if the user saves
|
||||
first, save what is known. Cancel it when the sheet closes.
|
||||
* "Edit details" -> the previous wave's add screen, pre-filled with the draft
|
||||
(title, subtitle, authors, publisher, year, pages, ISBN, description, cover URL if
|
||||
the draft supports it). If the add route cannot carry the full draft, extend it
|
||||
in `ui/add/` + `ui/nav/` minimally and consistently with how that wave built it
|
||||
(e.g. a serialized, URL-encoded draft argument, or a small draft holder). Say
|
||||
which you chose.
|
||||
- Keep the online search state in the ViewModel so it survives rotation.
|
||||
- Split stateless content composables so Paparazzi renders the REAL UI (see
|
||||
HANDOFF.md wave-6 "soft spots": a hand-rolled lookalike snapshot is not coverage).
|
||||
|
||||
## Constraints — these are hard
|
||||
|
||||
- Kotlin. Match the surrounding code's style, naming and comment density — this
|
||||
codebase comments the evidence for a decision, not a restatement of the code.
|
||||
- You MAY touch: `data/metadata/`, `ui/library/`, `ui/add/`, `ui/nav/`,
|
||||
`ui/components/` (sharing only), `AppContainer.kt` (wiring only), and if you choose
|
||||
the SQL route, `data/local/BookDao.kt` + `data/repo/BookRepository.kt` search
|
||||
methods only. Plus tests. Nothing else.
|
||||
- DO NOT touch any build file. Every library you need is already declared
|
||||
(OkHttp, kotlinx-serialization, coil3, Room, Compose). If you think you need
|
||||
something else, STOP and say so.
|
||||
- DO NOT touch `data/remote`, `data/prefs`, sync code, `server/`, `docs/`.
|
||||
- DO NOT read, print or grep `app/local.properties`; never put a real API key in code,
|
||||
tests or your report. Tests use "test-key-123". Do NOT make live network calls from
|
||||
unit tests (an opt-in live test gated on an env var like the existing
|
||||
`LiveMetadataLookupTest` is welcome but not required).
|
||||
- DO NOT git commit, add or push.
|
||||
- Build with `./tasks/gw <task>` — NEVER `./gradlew`.
|
||||
- RUN BUILDS IN THE FOREGROUND. Never background a build and end your turn promising a
|
||||
later report — you will never get to give it. Previous workers did exactly this.
|
||||
- No emulator. You cannot run the app. Do not claim you did.
|
||||
|
||||
## Definition of done
|
||||
|
||||
Run in the foreground and paste real output tails:
|
||||
./tasks/gw assembleDebug -> exit 0
|
||||
./tasks/gw testDebugUnitTest -> exit 0. Count MUST GO UP from whatever the
|
||||
previous wave left (sum TEST-*.xml under
|
||||
app/app/build/test-results/testDebugUnitTest/
|
||||
BEFORE you change anything, and again at the end).
|
||||
./tasks/gw recordPaparazziDebug -> exit 0
|
||||
./tasks/gw verifyPaparazziDebug -> exit 0
|
||||
|
||||
Required tests with REAL assertions:
|
||||
- OL parse from a fixture shaped like the real responses above (mixed ISBN-10/13,
|
||||
missing isbn, null cover_i); GB parse with null industryIdentifiers and no
|
||||
imageLinks; the OL request URL contains `fields=` including `isbn`; the query is
|
||||
URL-encoded ("a&b c" -> not a broken URL); GB URL carries the key only when
|
||||
non-blank; GB failure reasons are redacted.
|
||||
- Each search client: HTTP error -> Failed not NotFound; empty docs -> NotFound;
|
||||
throwing interceptor -> Failed(UNEXPECTED), class name only; CancellationException
|
||||
propagates.
|
||||
- Local matcher: "hobbit tolkien" matches The Hobbit by J.R.R. Tolkien;
|
||||
"jrr tolkien" and "tolkien hobbit" too; diacritics ("bronte" ~ "Brontë");
|
||||
a query token matching nothing excludes the book.
|
||||
- Merger: GB edition joins OL work by shared ISBN; "Joanne S. Williamson" and
|
||||
"Joanne Small Williamson" works collapse; "The Hobbit" x5 GB editions + OL work ->
|
||||
ONE online entry; an online hit of a DIFFERENT edition of an owned book lands in
|
||||
inLibrary, not online; transitivity (A~B by ISBN, B~C by title -> one group);
|
||||
ranking interleaves sources by best rank; distinct books with the same title but
|
||||
different authors stay separate.
|
||||
- ISBN rule, each branch: OL ["1883937388","9781883937386"] -> that ISBN-13; OL with
|
||||
many ISBNs + one GB ISBN -> GB's; several GB ISBNs and no single OL ISBN -> null;
|
||||
OL with no isbn field -> falls to GB or null.
|
||||
- ViewModel: typing does NOT trigger an online search; submit does; editing the query
|
||||
clears online results and cancels the in-flight search; one source failing still
|
||||
shows the other's results with a failure line; both failing is not reported as
|
||||
"no results"; save maps fields to createBook; a throwing save keeps the sheet with
|
||||
an error and does not throw.
|
||||
- Paparazzi light + dark: search with both sections populated; online loading while
|
||||
local results show; one source failed; the online-result save sheet.
|
||||
|
||||
Grep the build output for Kotlin warnings on files you touched; "Check for instance is
|
||||
always 'false'" in particular is a real bug signal on this project, not noise.
|
||||
|
||||
## Report
|
||||
|
||||
End with a plain report:
|
||||
- what you changed, file by file
|
||||
- verbatim tails of each gradle command
|
||||
- test count before and after, summed from the XML
|
||||
- how the draft reaches the add screen, and any change you made to the previous
|
||||
wave's route
|
||||
- your own judgement: where will dedupe get it WRONG on a real shelf (false merges
|
||||
and missed merges)? Name concrete cases. That is worth more than a clean report.
|
||||
- anything you could NOT do or did differently, and why
|
||||
- anything that looks wrong but was out of scope
|
||||
|
||||
Be honest. Workers here have over-claimed before, and everything is re-verified.
|
||||
Executable
+162
@@ -0,0 +1,162 @@
|
||||
#!/usr/bin/env bash
|
||||
# wave-chain.sh <wave>:<task> [<wave>:<task> ...]
|
||||
# e.g. wave-chain.sh 9:K-manual 10:L-search
|
||||
#
|
||||
# Runs waves IN SEQUENCE with no orchestrator attached (2026-09-13: the user asked for
|
||||
# wave 10 to start straight after wave 9 without waiting on them). Between waves it
|
||||
# runs a mechanical acceptance gate, because starting a worker on a red tree wastes a
|
||||
# whole session. This gate is NOT the orchestrator's review — it is the floor below it.
|
||||
#
|
||||
# Per wave:
|
||||
# 1. run-task.sh <task> in the FOREGROUND (quota-aware, resumable)
|
||||
# 2. gate: assembleDebug, testDebugUnitTest, verifyPaparazziDebug, test count UP from
|
||||
# the XML, 0 failures, no build files touched, no "always 'false'" warning
|
||||
# 3. gate red -> ONE fresh fix worker (<task>-fix) pointed at the gate log, re-gate
|
||||
# 4. green -> commit + push; red -> write sentinel as FAILED and STOP the chain
|
||||
#
|
||||
# Holds the sprite lease itself (HAZARD #5). wave-guard.sh cannot be used: its
|
||||
# workers_running() sees nothing during the 5-10 min gate between workers and would
|
||||
# release the lease mid-chain. Liveness here is `kill -0` on this script's own PID,
|
||||
# never pgrep -f (HAZARD #6/#10).
|
||||
set -u
|
||||
cd "$HOME/bookshelf" || exit 1
|
||||
L="$HOME/bookshelf/logs"; mkdir -p "$L"
|
||||
CLOG="$L/wave-chain.log"; LEASE="bookshelf-wave"
|
||||
RESULTS="app/app/build/test-results/testDebugUnitTest"
|
||||
|
||||
log() { echo "[$(date -Is)] $*" >> "$CLOG"; }
|
||||
|
||||
lease_present() { sprite-env curl /v1/tasks 2>/dev/null | grep -q "\"$LEASE\""; }
|
||||
lease_acquire() {
|
||||
local a
|
||||
for a in 1 2 3; do
|
||||
sprite-env curl -X DELETE "/v1/tasks/$LEASE" >/dev/null 2>&1
|
||||
sprite-env curl -X POST /v1/tasks -H 'Content-Type: application/json' \
|
||||
-d "{\"name\":\"$LEASE\",\"expire\":\"3600s\"}" >/dev/null 2>&1
|
||||
lease_present && return 0
|
||||
sleep 5
|
||||
done
|
||||
return 1
|
||||
}
|
||||
lease_release() { sprite-env curl -X DELETE "/v1/tasks/$LEASE" >/dev/null 2>&1; }
|
||||
|
||||
CHAIN_PID=$$
|
||||
(
|
||||
last=$(date +%s)
|
||||
lease_acquire && log "lease acquired and VERIFIED" || log "FATAL: lease acquire failed"
|
||||
while kill -0 "$CHAIN_PID" 2>/dev/null; do
|
||||
sleep 30
|
||||
now=$(date +%s)
|
||||
if [ $((now - last)) -ge 900 ] || ! lease_present; then
|
||||
lease_acquire && log "lease renewed" || log "ERROR: lease renewal FAILED"
|
||||
last=$now
|
||||
fi
|
||||
done
|
||||
) &
|
||||
RENEWER=$!
|
||||
trap 'kill $RENEWER 2>/dev/null; lease_release; log "chain exiting, lease released"' EXIT
|
||||
|
||||
count_tests() {
|
||||
python3 - "$RESULTS" <<'PY'
|
||||
import glob, sys, xml.etree.ElementTree as ET
|
||||
t = s = f = 0
|
||||
for p in glob.glob(sys.argv[1] + "/TEST-*.xml"):
|
||||
r = ET.parse(p).getroot()
|
||||
t += int(r.get("tests")); s += int(r.get("skipped")); f += int(r.get("failures")) + int(r.get("errors"))
|
||||
print(t, s, f)
|
||||
PY
|
||||
}
|
||||
|
||||
# gate <label> <baseline-test-count> -> 0 green, 1 red. Output in logs/gate-<label>.log
|
||||
gate() {
|
||||
local label="$1" base="$2" g="$L/gate-$1.log" ok=0
|
||||
: > "$g"
|
||||
echo "=== gate $label $(date -Is) baseline=$base ===" >> "$g"
|
||||
for t in assembleDebug testDebugUnitTest verifyPaparazziDebug; do
|
||||
echo "--- ./tasks/gw $t ---" >> "$g"
|
||||
./tasks/gw "$t" >> "$g" 2>&1; rc=$?
|
||||
echo "--- $t exit $rc ---" >> "$g"
|
||||
[ $rc -ne 0 ] && ok=1
|
||||
done
|
||||
read -r n s f <<<"$(count_tests)"
|
||||
echo "tests=$n skipped=$s failures+errors=$f (baseline $base)" >> "$g"
|
||||
{ [ "$f" -ne 0 ] || [ "$n" -le "$base" ]; } && { echo "GATE: test count did not go up or failures present" >> "$g"; ok=1; }
|
||||
if grep -n "always 'false'" "$g" >> "$g.warn"; then
|
||||
echo "GATE: \"always 'false'\" warning present:" >> "$g"; cat "$g.warn" >> "$g"; ok=1
|
||||
fi
|
||||
rm -f "$g.warn"
|
||||
echo "--- git status --porcelain ---" >> "$g"; git status --porcelain >> "$g"
|
||||
if git status --porcelain | grep -qE '\.gradle\.kts$|libs\.versions\.toml$|gradle\.properties$|local\.properties$'; then
|
||||
echo "GATE: a build file was touched (forbidden)" >> "$g"; ok=1
|
||||
fi
|
||||
echo "=== gate $label result: $([ $ok -eq 0 ] && echo GREEN || echo RED) ===" >> "$g"
|
||||
return $ok
|
||||
}
|
||||
|
||||
log "chain start: $*"
|
||||
read -r BASE _ _ <<<"$(count_tests)"
|
||||
log "baseline tests from XML: $BASE"
|
||||
|
||||
for spec in "$@"; do
|
||||
W="${spec%%:*}"; T="${spec#*:}"; SENT="$L/WAVE$W-DONE"
|
||||
log "wave $W ($T): launching worker"
|
||||
./tasks/run-task.sh "$T" "./tasks/$T.txt"
|
||||
log "wave $W ($T): worker exited: $(tail -1 "$L/$T.state")"
|
||||
|
||||
if gate "$T" "$BASE"; then
|
||||
GREEN=1
|
||||
else
|
||||
log "wave $W ($T): gate RED; launching one fix worker"
|
||||
cat > "tasks/$T-fix.txt" <<EOF
|
||||
A previous worker implemented the task in ~/bookshelf/tasks/$T.txt and left the
|
||||
working tree FAILING its acceptance gate. Read, in order: ~/bookshelf/docs/SPEC.md,
|
||||
~/bookshelf/tasks/$T.txt (the original instructions — every constraint in it binds
|
||||
you too), ~/bookshelf/logs/$T.summary (that worker's report), then the gate output at
|
||||
~/bookshelf/logs/gate-$T.log. Lines starting "GATE:" and non-zero exits are the failures.
|
||||
|
||||
Fix ONLY what makes the gate red. Do not redesign or re-implement the feature. The
|
||||
test count must end ABOVE $BASE with zero failures; do not delete or @Ignore tests to
|
||||
get there. If a Paparazzi verify failed because snapshots were never recorded, record
|
||||
them and LOOK at the PNGs before accepting them.
|
||||
|
||||
Run in the FOREGROUND (never background a build): ./tasks/gw assembleDebug,
|
||||
./tasks/gw testDebugUnitTest, ./tasks/gw verifyPaparazziDebug. Do not git commit/add/push.
|
||||
Report what was wrong, what you changed, and the verbatim output tails.
|
||||
EOF
|
||||
./tasks/run-task.sh "$T-fix" "./tasks/$T-fix.txt"
|
||||
log "wave $W ($T-fix): worker exited: $(tail -1 "$L/$T-fix.state")"
|
||||
if gate "$T-refix" "$BASE"; then GREEN=1; else GREEN=0; fi
|
||||
fi
|
||||
|
||||
if [ "$GREEN" -ne 1 ]; then
|
||||
log "wave $W ($T): gate still RED after fix worker. STOPPING CHAIN."
|
||||
{ echo "=== WAVE $W ($T) FAILED GATE $(date -Is) — chain stopped, later waves NOT run ==="
|
||||
echo "Work left UNCOMMITTED in the tree. See logs/gate-$T*.log, logs/$T*.summary."; } > "$SENT"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
read -r BASE _ _ <<<"$(count_tests)"
|
||||
COST="$(jq -r '"$" + ((.total_cost_usd//0)|tostring) + ", " + ((.num_turns//0)|tostring) + " turns"' "$L/$T.json" 2>/dev/null)"
|
||||
git add -A
|
||||
git commit -q -F - <<EOF
|
||||
Wave $W ($T): $( [ "$T" = K-manual ] && echo "add a book by hand, no ISBN required" || echo "search own library + Open Library + Google Books, deduplicated" )
|
||||
|
||||
Committed by tasks/wave-chain.sh after its mechanical gate passed
|
||||
(assembleDebug, testDebugUnitTest = $BASE tests, verifyPaparazziDebug, no build
|
||||
files touched, no "always 'false'"). ORCHESTRATOR REVIEW STILL PENDING.
|
||||
Prompt: tasks/$T.txt. Worker: $COST.
|
||||
|
||||
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||||
Claude-Session: https://claude.ai/code/session_01RWoivrRUEmJLFFwbsrGkqQ
|
||||
EOF
|
||||
SHA="$(git rev-parse --short HEAD)"
|
||||
if timeout 120 git push -q origin HEAD 2>>"$CLOG"; then PUSHED="pushed"; else PUSHED="PUSH FAILED (see wave-chain.log)"; fi
|
||||
log "wave $W ($T): GREEN, committed $SHA, $PUSHED, tests now $BASE"
|
||||
{ echo "=== WAVE $W ($T) passed the chain gate $(date -Is) ==="
|
||||
echo "commit $SHA ($PUSHED); tests $BASE; worker $COST"
|
||||
echo "Mechanical gate only. Orchestrator must still review: read logs/$T.summary,"
|
||||
echo "eyeball new Paparazzi PNGs, and read the diff (git show --stat $SHA)."; } > "$SENT"
|
||||
done
|
||||
|
||||
log "chain complete"
|
||||
echo "chain complete $(date -Is)" > "$L/CHAIN-DONE"
|
||||
Reference in new issue
Block a user