feat(amazon): resolve 979-prefix ISBN-13s via Creators API search_items - #13318
Open
openlibrary-bot wants to merge 1 commit into
Open
feat(amazon): resolve 979-prefix ISBN-13s via Creators API search_items#13318openlibrary-bot wants to merge 1 commit into
openlibrary-bot wants to merge 1 commit into
Conversation
979-prefix ISBN-13s could never be looked up on Amazon. They have no ISBN-10 equivalent, and `get_items` is an exact-id lookup that only accepts an ISBN-10 or a real ASIN, so Amazon (which assigns these books an arbitrary B* ASIN) was unreachable for the entire ISBN range. `Submit.GET` computed `key = isbn_10 or b_asin` -> None and exited to Google Books or `rejected_isbn` before any Amazon call. Keyword search is the only way to reach them. Adds `AmazonCreatorsAPI.search_items()` and `get_product_by_isbn_13()`, and routes only the previously-dead path through them, so nothing that works today can regress. A keyword search is not an exact-match lookup, so a result is accepted only if its own `external_ids.eans` contains the ISBN we asked for; otherwise we would cache and import the wrong book under the right ISBN. Latency is kept off the request path (#13277): resolution happens in the background `amazon_lookup` worker, and a high-priority 979 request still answers from Google Books immediately rather than waiting on the extra round trip. The Amazon result lands in the cache for the next request. `process_amazon_batch` maps each search-resolved product's ASIN back to the ISBN-13 that was queued for it. Without that, the stage_import filter compares `amazon:{B-ASIN}` against a queued ISBN-13, never matches, and stages items that asked not to be staged. Closes #13316
mekarpeles
marked this pull request as ready for review
August 12, 2026 11:39
|
👀 Code review was requested via its own PR being opened (self-authored, cannot request itself as reviewer), but the automated reviewer isn't built yet — tracked in #13163. This comment only confirms the trigger fired correctly. No review was performed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Makes 979-prefix ISBN-13s reachable on Amazon for the first time. Today they are silently dropped before any Amazon call: no price, no affiliate link, no import metadata — for the entire 979 range.
Closes #13316.
Why they were unreachable
get_itemsis an exact-id lookup accepting only an ISBN-10 or a real ASIN. A 979 ISBN-13 has no ISBN-10 equivalent, and Amazon assigns these books an arbitraryB*ASIN with no algorithmic relationship to the ISBN. So no conversion can find them — only a keyword search can.Submit.GETcomputedkey = isbn_10 or b_asin→Noneand exited to Google Books or{"error": "rejected_isbn"}before reaching Amazon. The Amazon branch was dead code for this ISBN range.Same root fact
amazon_affiliate_url()already documents from #6572 (per @hornc) — that fix handled the outbound link; the metadata lookup was never addressed.The change
AmazonCreatorsAPI.search_items()— the Creators API SearchItems operation, sharingget_products's throttle discipline and its fail-soft contract. (The wrapper had no search method; legacyAmazonAPI.search()was CLI-only and went away with PA-API in feat(amazon): require Creators API, remove PA-API fallback from affiliate server #13315.)get_product_by_isbn_13()— resolves an ISBN-13 with no ISBN-10 equivalent to its Amazon product.rejected_isbntoday goes through search. Strictly additive — nothing that currently works changes.Two existing behaviours meant the cache and import layers needed no changes:
serialize()already sourcesisbn_13fromexternal_ids.eansrather than deriving it from the ASIN, andmake_cache_key()already prefersisbn_13— so aB*-ASIN product caches under its true 979 ISBN-13, exactly the keySubmit.GETreads back.Two things worth reviewer attention
Search results are verified before use. A keyword search is not an exact-match lookup — Amazon may return a different edition, a boxed set, or something unrelated. A result is accepted only if its own
external_ids.eanscontains the requested ISBN; otherwise we would cache and import the wrong book under the right ISBN. This is the most important safety property here, and it's mutation-tested (breaking the check fails 3 tests).Latency is kept off the request path (#13277). Resolution runs in the background
amazon_lookupworker, never inline inSubmit.GET. A high-priority 979 request still answers from Google Books immediately rather than waiting on the extra round trip; the Amazon result lands in the cache for the next request. Notesearch_itemsis one ISBN per call with no batching, unlikeget_items(10/call) — so this is deliberately confined to identifiers that have no other way to be resolved.A regression this change would have introduced, and how it's handled.
process_amazon_batchfilters staging by comparingsource_records[0]against the queued identifiers. For a search-resolved product that'samazon:{B-ASIN}, while the caller queued the ISBN-13 — they never compare equal, so astage_import=falserequest would have been staged anyway. Resolved products are now mapped back to their originating identifier. Also mutation-tested.Testing
89 passedacrossopenlibrary/tests/core/test_vendors.pyandscripts/tests/test_affiliate_server.py— 11 + 7 new.Covered: happy path, serialization, empty-vs-error results, throttling, verified match accepted, wrong book rejected, correct item picked from mixed results, missing
eansrejected, search error →None, partitioning (mixed / search-only / ISBN-10-only /B*-only batches), cache key, and bothstage_importdirections.Regression guards assert 978 ISBNs and
B*ASINs never trigger a search.All pre-commit hooks pass, including
mypyandgenerate-pot(submodule initialized). mypy caught a real type error during development — the new guard didn't narrowisbn_13— which is fixed.Pre-existing on master and unrelated to this PR: 4 failures in
openlibrary/tests/core/test_fulltext.py::test_pagination_offset_calculation, confirmed by running that file on a clean checkout.No browser verification — this change has no frontend surface.
I have no Creators API credentials and no
ol-home0access, so I could not confirm Amazon actually returns the right item for a 979 ISBN, or at what hit rate. The code fails safe if it doesn't — an unverified result is rejected, so the worst case is a wasted background API call per 979 ISBN, not bad data. But the hit rate determines whether this is worth its cost, and that should be checked before merge.Our own test suite supplies a known-good input with a known-correct answer (
9798776159572→B09MJ3TKX3, from the #6572 work):If that comes back empty across a sample of 979 ISBNs, this should be closed rather than merged — see the discussion on #13316.