From 7c8e89c567ba4f7b1ba1e9d167920042831d9526 Mon Sep 17 00:00:00 2001 From: splitsec2 <35583321+splitsec2@users.noreply.github.com> Date: Sun, 20 Sep 2026 21:05:48 -0600 Subject: [PATCH] fix(googlebooks): page by the capped size, not the raw limit (#1370) The Google Books search builds `maxResults` as `min(limit, 40)` because the API caps a page at 40 volumes, but advances `startIndex` by the full `limit`. The pages then stop tiling. With `limit=50`, page 1 covers items 0 to 39 and page 2 starts at 50, so items 40 to 49 are never returned and every later page drops another 10. This computes the page size once and uses it for both `maxResults` and `startIndex`. The shipped frontend asks for 40 and is unaffected. `/api/metadata/search` clamps `limit` to 100, so an API caller passing 41 to 100 was hitting it. One thing I left alone. The provider uses the base `search_paginated` heuristic, `has_more = len(books) >= options.limit`, which still reports `has_more: false` for a limit above 40 since Google can never return that many. That was already the behaviour before this change, and fixing it means either touching the shared heuristic or adding a provider override, so I kept this patch to the stride. Happy to follow up if you want it. ## Verification - `tests/metadata/test_googlebooks_parse.py`: pages 1 and 2 at `limit=50` must tile exactly, plus a guard that `limit=25` still strides by 25. The first fails on current main and passes here. - Full suite (3165), ruff, ruff format, basedpyright, vulture green. --- shelfmark/metadata_providers/googlebooks.py | 5 +-- tests/metadata/test_googlebooks_parse.py | 36 +++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/shelfmark/metadata_providers/googlebooks.py b/shelfmark/metadata_providers/googlebooks.py index 72929d28..86b8174a 100644 --- a/shelfmark/metadata_providers/googlebooks.py +++ b/shelfmark/metadata_providers/googlebooks.py @@ -158,10 +158,11 @@ class GoogleBooksProvider(MetadataProvider): query = "+".join(query_parts) # Build request params + page_size = min(options.limit, 40) # Google max is 40 params: dict[str, Any] = { "q": query, - "maxResults": min(options.limit, 40), # Google max is 40 - "startIndex": (options.page - 1) * options.limit, + "maxResults": page_size, + "startIndex": (options.page - 1) * page_size, "printType": "books", # Exclude magazines } diff --git a/tests/metadata/test_googlebooks_parse.py b/tests/metadata/test_googlebooks_parse.py index 9557e724..2a7ad661 100644 --- a/tests/metadata/test_googlebooks_parse.py +++ b/tests/metadata/test_googlebooks_parse.py @@ -39,6 +39,15 @@ class _FlakyGoogleBooksSession: ) +class _RecordingGoogleBooksSession: + def __init__(self): + self.params = [] + + def get(self, *args, **kwargs): + self.params.append(dict(kwargs["params"])) + return _GoogleBooksResponse({"items": []}) + + def test_googlebooks_search_does_not_cache_request_failures(): get_metadata_cache().clear() provider = GoogleBooksProvider(api_key="test-key") @@ -54,6 +63,33 @@ def test_googlebooks_search_does_not_cache_request_failures(): assert [book.title for book in result] == ["Recovered Book"] +def test_googlebooks_pages_tile_when_limit_exceeds_api_maximum(): + get_metadata_cache().clear() + provider = GoogleBooksProvider(api_key="test-key") + session = _RecordingGoogleBooksSession() + provider.session = session + + for page in (1, 2): + provider.search(MetadataSearchOptions(query="Dune", limit=50, page=page)) + + first, second = session.params + assert first["maxResults"] == 40 + assert first["startIndex"] == 0 + assert second["startIndex"] == first["startIndex"] + first["maxResults"] + + +def test_googlebooks_page_stride_follows_limit_below_api_maximum(): + get_metadata_cache().clear() + provider = GoogleBooksProvider(api_key="test-key") + session = _RecordingGoogleBooksSession() + provider.session = session + + provider.search(MetadataSearchOptions(query="Dune", limit=25, page=3)) + + assert session.params[0]["maxResults"] == 25 + assert session.params[0]["startIndex"] == 50 + + class TestGoogleBooksParseVolume: def test_parse_volume_returns_metadata_for_valid_payload(self): provider = GoogleBooksProvider(api_key="test-key")