Skip to content

fix(python): derive offset pagination _has_next instead of hardcoding True#16155

Merged
fern-support merged 2 commits into
mainfrom
patrick/python/fix-offset-pagination-has-next
Jun 1, 2026
Merged

fix(python): derive offset pagination _has_next instead of hardcoding True#16155
fern-support merged 2 commits into
mainfrom
patrick/python/fix-offset-pagination-has-next

Conversation

@patrickthornton
Copy link
Copy Markdown
Contributor

@patrickthornton patrickthornton commented Jun 1, 2026

Closes FER-10931.

Summary

The Python SDK generator's offset paginator hardcoded _has_next = True for every generated list endpoint, which caused a couple problems: while pager.has_next: pager = pager.next_page() never terminated; for x in pager worked only because the core paginator stops when get_next() returns an empty page.

It also ignored the IR's optional OffsetPagination.has_next_page response property, so users who explicitly declared has-next-page: $response.<field> still got the hardcoded True.

Reported by a customer using offset pagination via x-fern-pagination: { offset: $request.page, results: $response.data }.

Change

init_has_next() in generators/python/.../pagination/offset.py now:

def init_has_next(self) -> str:
    if self.offset.has_next_page is not None:
        path = self._response_property_to_dot_access(self.offset.has_next_page)
        return f"bool({Paginator.PARSED_RESPONSE_VARIABLE}.{path})"
    return f"len({Paginator.PAGINATION_ITEMS_VARIABLE} or []) > 0"

Items-length fallback matches the TypeScript generator's hasNextPage: (r) => (r?.data ?? []).length > 0.

Before / after (generated)

 _items = _parsed_response.data.users if _parsed_response.data is not None else []
-_has_next = True
+_has_next = len(_items or []) > 0
 _get_next = lambda: self.list_with_offset_pagination(page=page + 1, ...)

And for endpoints that explicitly declare has-next-page: $response.hasNextPage:

-_has_next = True
+_has_next = bool(_parsed_response.has_next_page)

Test plan

  • pnpm seed test --generator python-sdk --fixture pagination --skip-scripts --local regenerated all 3 offset-pagination fixtures; snapshots updated in this PR.
  • Confirmed no remaining _has_next = True strings in seed/python-sdk/.
  • CI seed run.

… True

The Python SDK generator's offset paginator emitted `_has_next = True`
unconditionally, so `while pager.has_next: pager = pager.next_page()`
loops never terminated. It also ignored the IR's optional
`OffsetPagination.has_next_page` response property — even users who
explicitly declared `has-next-page` got the same hardcoded `True`.

`init_has_next()` now:
- emits `bool(_parsed_response.<path>)` when `has-next-page` is set on
  the pagination config, and
- falls back to `len(_items or []) > 0` otherwise, matching the
  TypeScript generator's `(r?.data ?? []).length > 0` semantics.
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@patrickthornton patrickthornton self-assigned this Jun 1, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 1, 2026

SDK Generation Benchmark Results

Comparing PR branch against median of 5 nightly run(s) on main (latest: 2026-06-01T05:35:53Z).

Full benchmark table (click to expand)
Generator Spec main (generator) main (E2E) PR (generator) Delta
python-sdk square 137s (n=5) 236s (n=5) 139s +2s (+1.5%)

main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via fern generate). main (E2E): full customer-observable time including build/test scripts (nightly baseline, informational). Delta is computed against generator-only baseline.
⚠️ = generation exited with a non-zero exit code (timing may not reflect a successful run).
Baseline from nightly runs on main (latest: 2026-06-01T05:35:53Z). Trigger benchmark-baseline to refresh.
Last updated: 2026-06-01 18:22 UTC

…l responses

Two latent crashes in the offset paginator, surfaced by code review:

1. Nested `has-next-page` paths dereferenced intermediate segments with no
   None-guard. `has-next-page: $response.metadata.hasNext` with a null
   `metadata` raised AttributeError. `get_next_none_safe_condition()` now
   returns the same null-guard expression `CursorPagination` uses, so the
   block runs under `if _parsed_response.metadata is not None:`.

2. `init_custom_vars_pre_next` was a no-op, so endpoints with an optional
   response type left `_has_next`/`_get_next` unbound when the server
   returned a null body — `SyncPager(has_next=_has_next, ...)` then raised
   NameError. Now pre-initializes both to `False`/`None` (matching cursor),
   so the conditional guard always has fallback values.

Neither path is exercised by current seed fixtures (no nested `has-next-page`
in test definitions, and no offset endpoint with `response: optional<T>`),
so snapshots are unchanged.
@fern-support fern-support self-requested a review June 1, 2026 18:14
@fern-support fern-support merged commit e90cdde into main Jun 1, 2026
77 checks passed
@fern-support fern-support deleted the patrick/python/fix-offset-pagination-has-next branch June 1, 2026 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants