feat(search): paginate search results with --pages N

[soimy]

Closes #29
Depends on #28 (stacked: this branch is based on fix/browser-cookie-dual-domain)

What

goofish search items gains --pages N (default 1, fully backward compatible) to walk the search pagination control and accumulate results across pages:

goofish search items "X570" --pages 5 --limit 200

Why the old approach couldn't page

Probed the live search page (2026-08, Playwright + system Chrome):

• The page is not infinite scroll: on a narrow query, scrolling to the bottom 3x stops growing cards (count pinned at 27, scrollHeight frozen) — the existing auto_scroll is a no-op past the first screen.
• Wide queries (X570) render a real pagination control: search-pagination-container with page boxes 1 2 3 … 50, 30 cards per page.
• Clicking the right arrow re-renders results in place — the URL stays ?q=X570. Measured page1/2/3 had zero card overlap.
• URL params ?page=2 / ?pageNumber=2 are ignored by the server (still return page 1).

So the only working pagination path is clicking the arrow, which the old implementation never did — limit 50 could only ever cover ~1/50 of the results.

Implementation

• --pages N: click right arrow → wait for SPA re-render by watching the first card id change (8s cap, waitForFunction-style predicate) + 2s settle → extract → dedupe by item_id → accumulate.
• Stop conditions (whichever first): --pages reached · right arrow disabled (last page) · limit reached · a page renders zero new cards (defensive against failed re-renders).
• limit is repurposed as the cross-page total cap (MAX_LIMIT 50 → 200); default 20 unchanged, so pages=1 behaves exactly as before.
• Response adds pages_fetched / pages_total metadata.
• Transient auth-wall retry from fix(browser): seed cookies into both taobao and goofish domains for search page auth #28 is preserved for page 1; mid-walk failures degrade gracefully (return partial results instead of raising).

Tests

• New unit tests: _normalize_pages clamping, MAX_PAGES constant (190 passed total), ruff clean.

Real end-to-end verification

scenario	result
"X570" --pages 3 --limit 200	90 items / 90 unique, pages_fetched=3, pages_total=50, 11s
"X570 Taichi" --limit 50 (single-page query)	26 items, pages_fetched=1, pages_total=1 — stops at disabled arrow
"X570" --pages 2 --limit 45	stops at exactly 45 items across 2 pages, 45 unique

Bonus: pages_total=50 metadata gives callers the full result volume before deciding how deep to walk (useful for the price-research workflow that surfaced this gap).

[fancyboi999]

Requesting changes on 2a59a4a (reviewed as the feature-only delta from #28).

The pagination direction matches #29, but two runtime paths can still lose or truncate results.

[P1] Preserve already-fetched pages when a mid-walk Playwright operation fails.

The pagination-state, page-change, and extraction page.evaluate() calls are outside the click-only try block. If the SPA destroys the execution context during a page transition, the exception propagates and the caller loses the first-page results. This contradicts the stated contract that mid-walk failures return partial results. Please catch the relevant Playwright/timeout failures around the complete per-page transition and return the accumulated items with a warning/termination reason.

[P1] Use the actual current DOM first card as the next page-change anchor.

After deduplication, cur_first_id = _first_card_id(fresh) stores the first new item rather than the page's actual first card. If page 2 starts with an item repeated from page 1, the stored anchor differs from the DOM before page 3 is clicked. _WAIT_PAGE_CHANGE_JS can then resolve immediately on the unchanged page; extraction reads page 2 again, finds no fresh items, and stops before page 3.

Set the anchor from the unfiltered next-page payload, check the page-change wait result, and add deterministic tests for a repeated first card, wait timeout, mid-walk exception, cross-page limit, deduplication, and pages_fetched/pages_total metadata.

Local verification at this exact head: 190 passed, Ruff clean, npm package tests 3/3, pack check clean, and git diff --check clean. None of the current automated tests executes _run() or the pagination state machine.

Because this PR is stacked on #28, it should be rebased onto the corrected #28 head after that review is addressed.

[fancyboi999]

Requesting changes on dbb757e after reviewing the correction delta from 2a59a4a.

The two original pagination findings are substantially addressed: per-page exceptions preserve accumulated results, the transition anchor now uses the unfiltered page payload, and the deterministic state-machine coverage is much stronger. Three items remain before this stacked PR can be merged.

[P1, blocking] Rebase onto the merged #28 cookie-records implementation.

This branch still descends from the original bf9f4bf and does not contain #28's final 7dad6ab / merged main state. The current tree removes cookie_types.py and restores the previously rejected cookie behavior: flat storage, QR provenance loss, and broadcasting cookies to both .taobao.com and .goofish.com with a forced / path.

#28 is now squash-merged as a536c479. Please rebase this PR onto current main, resolve browser.py/search conflicts by retaining the merged records/domain/path architecture, and ensure the resulting feature-only diff contains only pagination work.

[P2] Define behavior for cards without a parseable numeric item ID.

fresh filters by _item_id_from_url(...) not in seen, but IDs are added to seen only when non-empty. Two pages containing the same ...?id=abc card therefore both append it. I reproduced a three-page walk returning URLs 100, abc, abc with pages_fetched=3.

Please either skip cards without the required stable item ID or use an explicit stable fallback key, then add a cross-page regression for this shape.

[P2] Treat a non-dict pagination-state result as a graceful mid-walk error.

The first page.evaluate(_PAGINATION_JS) is exception-wrapped, but its result is not type-checked before pag.get(...). Returning None or another unexpected shape raises AttributeError and discards the partial-result contract. I reproduced this directly. Add the same isinstance(..., dict) guard used for extraction and return stopped_reason="error" with accumulated items.

Current isolated gates are otherwise green: 202 passed, 0 skipped/warnings, Ruff, npm tests 3/3, pack check, and git diff --check. I did not run live-cookie pagination E2E against this head because its stale #28 base still contains the rejected cross-domain cookie broadcast. After the rebase and the two boundary fixes, I will run --pages 3, exact cross-page limit, uniqueness/metadata, and partial-failure verification before approval.

[fancyboi999]

Requesting one final correction on 2eb912f after the rebase, full review, and successful real multi-page CLI validation.

The pagination work itself is now verified:

• GitHub checks: all 5 successful;
• local suite: 210 passed, 0 skipped/warnings;
• Ruff, npm tests 3/3, pack check, and git diff --check: clean;
• real authenticated X570 --pages 3 --limit 200: 90 items / 90 unique IDs / pages 3/50 / ranks 1–90 / stopped_reason=pages_reached;
• real authenticated X570 --pages 2 --limit 45: exactly 45 unique items / pages 2/50 / stopped_reason=limit;
• real authenticated X570 Taichi --pages 5: 29 unique items / pages 1/1 / stopped_reason=last_page.

The previous partial-result, anchor, non-dict pagination, invalid-ID, limit, and metadata findings are addressed. One integration regression remains.

[P1, blocking] Preserve #28's exact auth-wall retry predicate after the rebase.

Merged main (a536c479) introduced _should_retry(payload) so the first-page retry runs only for items=[] together with requiresAuth=true. This PR deletes that helper and restores the broader condition that retries every zero-item state not marked empty or blocked.

I reproduced an unknown zero-item payload with requiresAuth=false: the current PR navigates twice before raising GoofishError. The merged #28 contract requires one attempt for that shape. Please restore _should_retry() from main, call it from the first-page loop, and retain its truth-table coverage while integrating the pagination tests.

[P2] Keep a public search() contract regression.

The rebase also removes main's top-level successful search() result test and auth-error propagation test; the new suite exercises _walk_pages directly. Because #28 previously lost _run()'s success return while all tests stayed green, please retain at least one fake-page-driven _run() / public search() test that asserts items, rank, query, pagination metadata, and error propagation.

Non-blocking cleanup: tests/test_search.py contains unused _run_walk / _get_walk helpers, and several permanent comments refer to historical “review P1/P2” labels instead of describing the mechanism directly.

Once the retry behavior and top-level regression are restored, I will rerun the focused suite and the same real multi-page CLI matrix; no further browser-logic blocker is currently known.

[fancyboi999]

Approved at 26dc074 after final incremental review and real CLI verification.

The remaining integration regression is resolved: _should_retry() matches merged #28, unknown zero-item responses without requiresAuth are not retried, and public search() success/default/error regression coverage is restored. The cookie records implementation from #28 is retained.

Verification at this exact head:

• Locked Python 3.11 environment: 215 passed with -W error, 0 skipped/warnings.
• Ruff, npm tests 3/3, package dry-run, and diff check: passed.
• All five GitHub checks passed, including Python 3.11/3.12 tests and build.
• Real CLI with isolated credentials from the authenticated ego-lite session: MTop auth status returned valid=true.
• X570 --pages 3 --limit 200: 90 items, 90 unique IDs, 3/50 pages, sequential ranks, pages_reached.
• X570 --pages 2 --limit 45: exactly 45 unique items, 2/50 pages, limit.
• X570 Taichi --pages 5 --limit 200: 27 unique items, 1/1 page, last_page.
• Partial-result failures, invalid-ID handling, pagination shape guards, and retry boundaries are covered by deterministic tests; real service failures were not deliberately induced.

Both Standards and Spec incremental reviews found no remaining actionable issues. No unresolved review threads remain.