feat(pagination): plumb max_pages through public *_all() methods

[TexasCoding]

Summary

Wave 2 #98: exposes the previously-internal _list_all pagination cap as a max_pages: int | None = None kwarg on every public *_all() resource method (sync + async), and removes the silent 1000-page hard cap so None is truly unbounded.

Scope

• kalshi/resources/_base.py — _list_all accepts max_pages: int | None = None (sync + async). The cap is now only applied when the caller passes an int; None iterates until the server returns no cursor. New _validate_max_pages helper raises ValueError at the public boundary on <= 0.
• 38 public method signatures plumbed (19 sync + 19 async) across 12 resource files.
• Eager-validation fix: 7 wrapper methods that used yield from self._list_all(...) (5 sync) or async for item in ...: yield item (2 async) were silently generators — their body didn't execute until first next(), so _validate_max_pages ran lazily. Converted to return self._list_all(...). The 2 async wrappers additionally switched from async def → def so they return the AsyncIterator immediately (otherwise the coroutine wrapper itself defers the body).
• tests/_contract_support.py — new ExclusionKind = "client_only" for SDK-only kwargs (vs. paginator_handled which describes spec params the SDK hides). All 38 FQNs (sync + async) registered in _MAX_PAGES_FQNS.
• tests/test_contracts.py — added client_only to sig_mismatch_kinds.

Behavior change (was the open question — now resolved)

Before: _list_all had an invisible 1000-page hard cap. Users iterating beyond ~100k items were silently truncated.

After: max_pages=None iterates until the server returns no cursor. The existing cursor-repeat guard (KalshiError on a repeating cursor value) provides the runaway protection — that's what stops infinite loops if a server bug emits stuck cursors. Users wanting a cap pass max_pages=N explicitly.

CHANGELOG [Unreleased] → Changed calls this out. The cap was never part of the public contract; the change matches user expectation for None and is the right long-term direction (silent truncation post-release is a worse compat problem to unwind later).

Cap-of-0

Rejected at the public boundary with ValueError("max_pages must be positive or None").

Test deltas

• TestValidateMaxPages — None / positive / zero / negative paths
• TestMaxPagesEagerValidation (×4) — sync milestones, sync subaccounts, async subaccounts (AsyncIterator-returning def), async communications. Each asserts ValueError fires at call time, not on first __next__.
• TestMaxPagesNoneIsUnbounded (×2) — sync + async. Server returns 1010 pages; suite iterates all 1010, proving the 1000 cap is gone in both code paths.
• Plus test_markets.py::test_max_pages_caps_fetch and test_max_pages_zero_rejected as end-to-end signal on the public surface.

Total: +17 tests over baseline.

Closes #98

Test plan

• Suite: 1577 passed
• uv run ruff check . clean
• uv run mypy kalshi/ clean (strict; 75 files)

[claude]

Code Review — PR #135: max_pages on public *_all() methods

Overall the implementation is clean and well-structured. The plumbing is mechanical and correct, the contract exclusion approach is sound, and test coverage is solid. I have one real bug, one API design concern the PR itself flags, and a couple of minor notes.

---

Bug: lazy validation in generator-based methods

_validate_max_pages runs eagerly only in methods that return self._list_all(...). In methods that use yield from (which makes them generator functions), the validator doesn't execute until the caller first advances the iterator — not at call time.

Affected sync methods:

• communications.CommunicationsResource.list_all_rfqs
• milestones.MilestonesResource.list_all
• incentive_programs.IncentiveProgramsResource.list_all
• subaccounts.SubaccountsResource.list_all_transfers

Affected async methods (those with async for item in ...: yield item patterns):

• communications.AsyncCommunicationsResource.list_all_rfqs
• subaccounts.AsyncSubaccountsResource.list_all_transfers

The test comment in test_markets.py says "The validator runs eagerly inside list_all, before the generator is consumed" — but that's only true for MarketsResource.list_all (which uses return). Calling milestones.list_all(max_pages=0) will not immediately raise; the ValueError only appears on first next().

Fix options:

1. Have each generator method call _validate_max_pages before the first yield — which already happens syntactically, but the issue is that Python doesn't run any generator body code until next() is called. To get eager behavior you'd need to split into a private helper: def list_all(...): _validate_max_pages(max_pages); return _list_all_impl(...)
2. Accept lazy validation and update docs/tests accordingly.
3. Convert the generator-based methods to return-based (the way markets.list_all works), since _list_all itself returns an Iterator.

Option 3 is the cleanest — just return self._list_all(...) works for sync and matches the non-generator methods. The yield from pattern isn't necessary here since _list_all already returns an Iterator.

---

API design: None → 1000 (silent truncation)

The PR description flags this clearly and asks for reviewer input. My take:

max_pages=None strongly implies "no cap" to most users. Someone who writes:

for trade in client.historical.trades_all():
    ...

will be silently capped at 100k trades. There's no warning, no exception, no indication that data was truncated.

Recommendation: make None truly unbounded. The existing cursor-repeat guard (KalshiError on repeated cursor) and the server's eventual empty-cursor response are already the real protection against infinite loops. The 1000-page cap was never part of the public contract — it was an internal implementation detail.

If a safety net is still wanted, a _DEFAULT_PAGE_CAP constant (e.g., 10_000) makes the internal value visible without hiding it behind None, and users who need unbounded can pass max_pages=math.inf or a sentinel. But simplest: None = unbounded, document it.

This is the one behavior choice worth reconsidering before merging, because fixing it post-release is a breaking change (removing a silent cap) that could break callers who rely on the bounded behavior — even if they don't know they do.

---

Missing async FQNs in _MAX_PAGES_FQNS

_MAX_PAGES_FQNS in _contract_support.py only lists sync resource FQNs (e.g., MarketsResource, EventsResource). The async counterparts (AsyncMarketsResource, AsyncEventsResource, etc.) are absent. Tests currently pass because drift tests appear to check sync methods only, but if coverage is ever extended to async classes, all 19 async methods would fail the drift check. Worth adding them now while the change is fresh.

---

Minor nits

Duplicate inline _make_response factory in test_markets.py:test_max_pages_caps_fetch — this is a near-copy of _fresh_cursor_side_effect() in test_base_helpers.py, just with "markets"/"ticker" keys instead of "items"/"id". Not a blocker, but it could share the base helper if the helper were parameterized on the response shape.

_validate_max_pages is not applied inside _list_all itself — this is the right call (it's an internal function), but worth noting for future maintainers that _list_all(max_pages=0) silently returns an empty iterator (correct for internal callers; surprising if called from outside the resource layer).

---

Summary

Bug	Lazy validation in yield from generators — _validate_max_pages doesn't fire at call time for ~6 methods
Design	None → 1000 silently truncates callers expecting unbounded iteration
Housekeeping	Missing async FQNs in _MAX_PAGES_FQNS; inline duplicate test helper
✅	Mechanical plumbing is correct, consistent, and well-tested for the non-generator paths
✅	ExclusionKind = "client_only" is the right way to handle this in the drift framework
✅	_validate_max_pages as a standalone helper at the public boundary is clean

Block on the silent-truncation question — that's a user-facing behavior decision. The lazy-validation bug should be fixed before merge.

[TexasCoding]

Both bot findings addressed; plus the related async generator issue that surfaced while fixing the sync one.

Fixed

• ⚠️ max_pages=None is now truly unbounded (your recommendation). Cursor-repeat guard remains the safety net; 1000-cap removed.
• 🔴 Lazy validation in yield from / async for ... yield methods — fixed via return self._list_all(...). Additionally found that the 2 async wrappers needed def (not async def) — otherwise the wrapper is a coroutine and validation still runs only on await. Switched both.
• 🟡 Missing async FQNs in _MAX_PAGES_FQNS — added all 19 async resource FQNs.

Regression coverage added

• TestMaxPagesEagerValidation × 4 — sync milestones, sync subaccounts, async subaccounts (AsyncIterator-returning def), async communications. Each asserts ValueError fires at call time, not on first __next__.
• TestMaxPagesNoneIsUnbounded × 1 — server returns 1100 pages; suite iterates all 1100, proving the 1000 cap is gone.

Skipped (justified)

• Duplicate _make_response test helper extraction — would touch tests outside this PR's scope; defer to follow-up cleanup.
• "_validate_max_pages not applied inside _list_all itself" — bot called this the right call; internal callers are trusted.

Verify: 1577 passed (+5 over agent baseline), ruff clean, mypy --strict clean.

[claude]

Code Review — PR #135: max_pages plumbed through *_all() methods

Overall this is a clean, well-executed wave. The async def → def refactor for eager validation is the right call and the test class TestMaxPagesEagerValidation nicely pins that invariant. A few items worth discussing before merge:

---

🔴 Behavioral Breaking Change — Needs CHANGELOG + Decision

The PR description already surfaces this, but it needs a resolution before merge.

Before: _list_all default was max_pages=1000 — an invisible 100k-item safety cap.
After: max_pages=None (public default) → truly unbounded. Only the cursor-repeat guard stops runaway iteration.

This is the right long-term direction — the old cap silently truncated data and was invisible to callers. But it's a behavioral change that needs:

1. A CHANGELOG.md entry under an upcoming release noting that *_all() is now unbounded by default (previously capped at 1000 pages).
2. A decision on the three options listed in the PR summary. My recommendation: ship as unbounded (option 2). The cursor-repeat guard already defends against server bugs. Users who want a cap now have a clean knob (max_pages=N). Silently capping at 1000 was surprising behavior for anyone pulling historical fills or trades at scale.

---

🟡 Test Performance — TestMaxPagesNoneIsUnbounded

# tests/test_base_helpers.py ~L1412
cursor = str(n) if n < 1100 else ""

This test makes 1100 mock HTTP requests to prove unboundedness. It proves the point but adds ~1s of overhead per run. Using 1010 instead of 1100 still proves "goes past the old 1000 cap" with 90% fewer requests. Consider dropping to 1010 (or even 1003) to keep the test suite snappy.

---

🟡 Order of Validation vs. Auth

Validation order is inconsistent across resources:

• portfolio.py, orders.py, fcm.py: _require_auth() → _validate_max_pages()
• events.py, markets.py, historical.py (public endpoints): _validate_max_pages() only (no auth needed, fine)

This is correct as-is since _require_auth raises KalshiError (not ValueError), so they're distinguishable. No change needed — just confirming this was intentional.

---

🟡 Async docstring lost the KalshiError note

# Before:
"""Async counterpart of ``SyncResource._list_all``. Raises
``KalshiError`` on repeated cursor; see sync docstring.
"""
# After:
"""Async counterpart of :meth:`SyncResource._list_all`."""

The cursor-repeat guard still exists in the async path. Worth keeping the behavior note so callers know to handle KalshiError from async iteration. Even one line would suffice: Raises ``KalshiError`` on repeated cursor.

---

🟡 _fresh_cursor_side_effect closure counter

# tests/test_base_helpers.py ~L1206
counter = {"n": 0}

def _make_response(request: httpx.Request) -> httpx.Response:
    counter["n"] += 1

The dict-as-counter workaround for the missing nonlocal is understood but slightly surprising in a test helper. A nonlocal n pattern or an itertools.count() would read more naturally. Minor — take or leave.

---

✅ What's done well

• _validate_max_pages placement: Right at the public boundary, before any I/O, with a clear error message including the bad value. Good.
• async def → def conversion: Correct and necessary so _require_auth + _validate_max_pages fire eagerly. The inline comment explaining why is exactly the right level of documentation per CLAUDE.md (non-obvious invariant).
• ExclusionKind("client_only"): Clean distinction from paginator_handled. The _MAX_PAGES_FQNS registration pattern makes the exclusion set explicit and auditable.
• while loop with pages_fetched: Cleaner than for _ in range(max_pages) and correctly handles None. No off-by-one concerns.
• Test coverage: TestValidateMaxPages, TestMaxPagesEagerValidation, and the base helper cap tests cover the meaningful cases. The test_markets.py additions anchor the public-API contract end-to-end.

---

Summary

Approve with the following before merge:

1. Add CHANGELOG entry for the default-unbounded behavior change.
2. Optionally reduce 1100 → 1010 in TestMaxPagesNoneIsUnbounded.
3. Restore the KalshiError note to the async _list_all docstring.

The core change is solid — plumbing is consistent, validation is eager, and the client_only exclusion kind is the right abstraction for this class of SDK-only kwarg.

[TexasCoding]

Second-pass items addressed in latest commit:

• 🔴 CHANGELOG entry — added [Unreleased] → Added (new max_pages kwarg + RateLimit export) and [Unreleased] → Changed calling out that *_all() is now unbounded by default (was silently capped at 1000).
• 🟡 Async docstring — restored the "Raises KalshiError on repeated cursor; see sync docstring." line that got trimmed.
• 🟡 Test perf — 1100 → 1010. Still exceeds the old cap by enough to prove unboundedness; 90% fewer mock requests.

Skipped (justified):

• nonlocal / itertools.count() vs dict-counter — bot called this take-or-leave; the dict-counter pattern is consistent with _fresh_cursor_side_effect already in the file. Keeping for consistency.

[claude]

Code Review — PR #135: max_pages on all *_all() methods

Overview

Clean, well-scoped feature: exposes the previously-internal 1000-page cap as an optional, user-controlled max_pages: int | None = None kwarg, while simultaneously removing the silent hard cap so None is truly unbounded. The eager-validation pattern (converting yield from/async for ... yield generators to return self._list_all(...) and async def → def) is the right call and is clearly documented.

---

Issues

Bug / Stale Documentation

_validate_max_pages docstring says "use default cap" — but there is no longer a default cap.

# kalshi/resources/_base.py (new helper)
def _validate_max_pages(max_pages: int | None) -> None:
    """...
    ``None`` (use default cap) and positive integers are valid.   # ← stale
    ...
    """

None now means unbounded, not "use a 1000-page cap." This docstring reads as if an internal 1000-page fallback still exists. Should be corrected to something like:

``None`` (unbounded — iterates until the server returns no cursor) and
positive integers are valid.

PR Description Misleads on the Core Behavior Change

The PR description's "⚠️ Behavior change" section reads:

None (default) → internal cap of 1000 pages. Before this PR, *_all() iterated unbounded; now an explicit max_pages=None is rewritten to 1000 inside _list_all.

This is the opposite of what the code does — before this PR the SDK silently capped at 1000; after, None is truly unbounded. The CHANGELOG entry is correct; the PR description body is not. Not a code issue, but worth correcting so it doesn't confuse future git log/gh pr list readers.

---

Test Coverage Gaps

No async counterpart for test_sync_iterates_past_1000_pages.

TestMaxPagesNoneIsUnbounded proves the old cap is gone for sync, but the same regression can silently re-appear in the async path. A one-line async sibling is cheap and closes the gap:

@respx.mock
@pytest.mark.asyncio
async def test_async_iterates_past_1000_pages(
    self, test_auth: KalshiAuth, test_config: KalshiConfig,
) -> None:
    total = 1010
    call_counter = {"n": 0}

    def responder(request: httpx.Request) -> httpx.Response:
        call_counter["n"] += 1
        n = call_counter["n"]
        cursor = str(n) if n < total else ""
        return httpx.Response(200, json={"items": [{"id": f"i{n}"}], "cursor": cursor})

    respx.get("https://test.kalshi.com/trade-api/v2/things").mock(side_effect=responder)
    resource = AsyncResource(AsyncTransport(test_auth, test_config))
    items = [item async for item in resource._list_all("/things", _Item, "items")]
    assert len(items) == total

---

Minor Suggestions

_fresh_cursor_side_effect return type annotation. The return type is Any; it could be Callable[[httpx.Request], httpx.Response] for accuracy, though this is test-only so low priority.

TestMaxPagesEagerValidation resource sampling. The test class covers 4 specific resources (milestones, subaccounts sync/async, communications async) but not the others. A brief comment explaining "these are representatives for the yield from → return and async def → def patterns" would help future readers understand why the sample was chosen.

---

What's Well Done

• Correct eager-validation pattern throughout. Every resource that previously used yield from or async for ... yield was correctly converted to return self._list_all(...) / plain def. The comment in communications.py and subaccounts.py explaining why def instead of async def is exactly the kind of non-obvious rationale that belongs in code.
• client_only exclusion kind is a precise fit — semantically distinct from paginator_handled (which hides a spec param) vs. client_only (which adds one). Good naming.
• 1010-page regression test explicitly proves the old cap is gone and explains the choice of 1010 in a comment.
• Contract test wiring is complete: all 38 FQNs are registered in _MAX_PAGES_FQNS and the sig_mismatch_kinds set is properly extended.
• Sync/async logic is fully symmetric — no drift between the two _list_all implementations.

---

Summary

Two items to address before merge:

1. Fix the stale _validate_max_pages docstring ("use default cap" → "unbounded").
2. Add an async counterpart to test_sync_iterates_past_1000_pages.

The PR description wording is worth fixing too, but it won't block the code from being correct. Everything else is solid.

[TexasCoding]

Third-pass items addressed in latest commit + PR-body rewrite:

• 🔴 Stale _validate_max_pages docstring — fixed. Now reads None (unbounded — iterates until the server returns no cursor) rather than the old use default cap.
• 🔴 Missing async counterpart for unbounded test — added test_async_iterates_past_1000_pages next to its sync sibling. Same 1010-page proof, prevents the regression from silently re-appearing in only one code path.
• ✅ PR description rewritten — the original "⚠️ Behavior change" section described the pre-fixup intent (None → 1000) and was now misleading. Rewrote to match what the code actually does + folded in the eager-validation explanation. CHANGELOG was already correct.

Skipped (non-blocking):

• _fresh_cursor_side_effect return type annotation — test-only utility, low value.
• TestMaxPagesEagerValidation comment about sample choice — adds bytes without adding signal; the class docstring already names the two patterns (yield from → return and async def → def).

[claude]

Code Review — PR #135: max_pages pagination cap

Overview

This PR exposes the previously-internal 1000-page pagination cap as a public max_pages: int | None = None kwarg on all 38 *_all() methods (19 sync + 19 async), and removes the silent hard cap so None is truly unbounded. The approach is sound: _validate_max_pages at the public boundary, while loop in _list_all, cursor-repeat guard remains the safety net.

---

Correctness

Loop logic is correct. The while max_pages is None or pages_fetched < max_pages: pattern handles both bounded and unbounded cases. The counter increments after yield from page.items and before the cursor check — right order.

Eager-validation fix is correct. Methods that previously used yield from self._list_all(...) were silently generators — validation deferred until first next(). Converting to return self._list_all(...) fixes this for sync methods. The async async def → def conversion is the correct pattern: the returned AsyncIterator from the async generator in _list_all is still fully functional; only the coroutine wrapper that was deferring body execution is gone.

One subtle correctness point to verify: In AsyncResource._list_all, the method stays async def with yield (it remains an async generator). The diff confirms this — only the wrappers changed to plain def. This is correct and intentional.

---

Potential Issues

1. Eager-validation test coverage is incomplete for converted methods.

The PR description mentions 7 methods were converted from yield from/async for … yield to return, but TestMaxPagesEagerValidation covers only 4 of them:

Method	Eager test?
MilestonesResource.list_all	✅
SubaccountsResource.list_all_transfers	✅
AsyncSubaccountsResource.list_all_transfers	✅
AsyncCommunicationsResource.list_all_rfqs	✅
IncentiveProgramsResource.list_all	❌
StructuredTargetsResource.list_all	❌
CommunicationsResource.list_all_rfqs (sync)	❌

Since all three use the same return self._list_all(...) pattern, the regression is structurally impossible, but adding the three missing cases would make the test class exhaustive and avoid any doubt in future audits. Low priority.

2. _validate_max_pages docstring violates project conventions.

CLAUDE.md says: "Never write multi-paragraph docstrings or multi-line comment blocks — one short line max." The helper has a 5-line docstring. Suggested trim:

def _validate_max_pages(max_pages: int | None) -> None:
    # Zero or negative would produce an empty iterator silently deep in _list_all.
    if max_pages is not None and max_pages <= 0:
        raise ValueError(f"max_pages must be positive or None, got {max_pages}")

3. Behavior change — silent truncation removed.

The removal of the 1000-page cap is technically a breaking change for any caller that relied on it as a safety budget (even inadvertently). The CHANGELOG calls it out clearly under ### Changed, which is the right place. Consider whether a [Unreleased] → Breaking section or semver bump note is needed if this SDK follows strict semver — removing a guarantee (even an undocumented one) typically warrants at least a minor version bump signal.

4. _fresh_cursor_side_effect naming.

The function returns a callable (closure) to use as a respx side_effect, not a side-effect directly. The name implies it returns the effect itself. Consider _make_fresh_cursor_responder or just inline the counter into each test class. Minor.

---

Performance

The TestMaxPagesNoneIsUnbounded tests make 1010 iterations. Since these use respx.mock (in-process), not real HTTP, this is fine. The inline comment explains the choice — appreciated.

---

Test Coverage

The new test suite is solid:

• TestValidateMaxPages — all four boundary cases (None, positive, zero, negative)
• TestMaxPagesEagerValidation — the tricky generator-deferral regression
• TestMaxPagesNoneIsUnbounded — proves the hard cap is gone in both paths
• TestSyncListAllMaxPagesCap / TestAsyncListAllMaxPagesCap — verifies the count
• test_max_pages_caps_fetch / test_max_pages_zero_rejected — end-to-end signal on the public surface

The empty-cursor regression guard (test_empty_cursor_stops_after_one_request) is a nice bonus.

---

Summary

Category	Finding
Correctness	✅ Logic is sound; eager-validation fix is right
Tests	✅ Good coverage; 3 eager-validation cases not yet exercised (low risk)
Conventions	⚠️ Multi-line docstring on _validate_max_pages
Breaking change	ℹ️ Documented, but verify semver intent
Code quality	✅ Clean, mechanical, follows existing patterns

Two action items worth addressing before merge:

1. Trim _validate_max_pages docstring to one line.
2. Optionally add the 3 missing eager-validation test cases for completeness.

Everything else looks ready.

[claude]

Code Review — PR #135: max_pages pagination cap

Status: LGTM with minor notes. The core design is correct and the implementation is thorough. Comments below are mostly nits/observations rather than blockers.

---

Overview

This PR exposes max_pages: int | None = None across all 38 public *_all() methods (19 sync + 19 async), removes the invisible 1000-page hard cap, and fixes a subtle eager-vs-lazy generator evaluation bug. The approach is sound and the test coverage is solid.

---

What's Done Well

Eager validation fix is the right call.
Converting async def list_all_rfqs/list_all_transfers to plain def returning AsyncIterator is the correct fix for the generator-body-deferral problem. Without this, _require_auth() and _validate_max_pages() would have been silently deferred until first __next__(). The in-code comments explaining why (# Plain def so _require_auth + _validate_max_pages run at call time) are exactly the kind of non-obvious WHY that warrants a comment per CLAUDE.md.

while loop replacement is clean.

# Before
for _ in range(max_pages):  # range(None) would crash

# After
while max_pages is None or pages_fetched < max_pages:
    ...
    pages_fetched += 1

Correctly handles both the bounded and unbounded cases without a sentinel value or special-casing.

pages_fetched incremented in the right place.
Incrementing after yield from page.items but before the cursor break means max_pages=N fetches exactly N pages. Traced through manually — the boundary conditions (max_pages=1, max_pages=3) are correct.

Contract test registration is complete.
All 38 FQNs in _MAX_PAGES_FQNS are individually listed (not a glob), which means adding a new resource won't silently hide drift. The new client_only exclusion kind is distinct from paginator_handled — good separation of semantics.

Test coverage is comprehensive.

• TestValidateMaxPages — covers all four code paths (None, positive, zero, negative)
• TestMaxPagesEagerValidation — spot-checks the specific methods that were generator-based
• TestMaxPagesNoneIsUnbounded — the 1010-page regression test is the most important one; it proves the old cap is gone in both sync and async paths
• test_markets.py end-to-end signal on the public surface

---

Minor Issues

1. Multi-line docstring on _validate_max_pages violates CLAUDE.md conventions.

def _validate_max_pages(max_pages: int | None) -> None:
    """Reject ``max_pages <= 0`` at the public ``*_all()`` boundary.

    ``None`` (unbounded — iterates until the server returns no cursor) and
    positive integers are valid. Zero or negative would silently produce an
    empty iterator deep inside ``_list_all``; surfacing the misuse here is
    friendlier.
    """

CLAUDE.md says "Never write multi-paragraph docstrings or multi-line comment blocks — one short line max." The body is useful context, but it belongs in the PR description (where it already lives) rather than the docstring. A one-liner like """Raise ValueError if max_pages is zero or negative.""" is sufficient.

2. CHANGELOG bundles an unrelated entry.

### Added
- **`RateLimit` model** exposed via `kalshi.RateLimit`...

This appears to be a leftover from a different change bundled into [Unreleased]. If RateLimit was added in a prior PR, it shouldn't be in the "Added" section of this PR's CHANGELOG entry — it creates confusion about what Wave 2 #98 actually introduced.

3. _fresh_cursor_side_effect uses a mutable-dict counter instead of a list.

counter = {"n": 0}  # workaround for Python closure rebinding
def _make_response(request):
    counter["n"] += 1

This is a common Python pattern to avoid nonlocal, but nonlocal is cleaner and more idiomatic in Python 3:

n = 0
def _make_response(request):
    nonlocal n
    n += 1

Functionally equivalent — just a readability nit.

4. The 1010-page tests make ~1010 mock HTTP calls each.

The sync and async "past 1000 pages" tests iterate 1010 pages. With respx mocking these are fast (~0.3–0.5s each), but if this test suite ever runs in a slow CI environment, consider reducing to 101 (just past the hypothetical cap) with a comment explaining the choice. The current 1010 is defensible; 101 would be equally effective at catching the regression.

---

No Issues Found In

• The loop termination logic (cursor-empty break, cursor-repeat guard)
• Sync/async symmetry across all 12 resource files
• _require_auth() placement relative to _validate_max_pages() (auth checked first — correct)
• Contract drift test integration (sig_mismatch_kinds updated correctly)
• Type annotations (int | None consistently used throughout)
• The multivariate.py case which uses return self._list_all(...) without an explicit return keyword previously — that was already correct and remains so

---

Summary

The design is right, the fix for lazy generator validation is correct, and the test suite properly proves the old cap is gone. The minor items above are style/cleanup nits that don't affect correctness. This is a well-scoped, well-executed wave of a larger pagination feature.