Repository navigation
Conversation
This branch has not been deployed
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 does this PR do?
Add regression coverage for the row-counting fix in #2101. A caller may clear a page's data after processing it to release memory. The iterator must still count those rows toward
max_items.Exercise both
Langfuse.iter_observation_pagesandLangfuse.aiter_observation_pageswith a three-item cap. The tests consume and clear each page, then verify that only three observation IDs are delivered and that the final request is limited to one remaining item. A further scripted page is available to expose an extra fetch.This is a test-only contribution. The current implementation already counts rows before yielding; runtime code is unchanged.
Type of change
Verification
The original pagination suite passes 10 cases; the expanded suite passes 12, including both new sync/async cases. Tests use the existing scripted API fixture and require no Langfuse server or model calls. The repository-wide suite, e2e tests and live-provider tests were not run for this single test-file change.
Checklist
code_review.md.

Confidence Score: 5/5
This test-only PR appears safe to merge.Summary
Adds sync and async regression tests for callers that clear
page.dataafter reading it.Reviews (1) · Last reviewed commit: "test(pagination): preserve item limits w..." · Reviewed by Greptile