fix(sessions): paginate OpenAI conversations session reads - #4956
fix(sessions): paginate OpenAI conversations session reads#4956Neonstudio28 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved review comments remain, and all reviewers assessed it as ready.
Pull request overview
Fixes OpenAI Conversations session pagination by applying limits locally while preserving chronological ordering.
Changes:
- Delegates provider pagination to the async iterator.
- Adds regression coverage for local limits and ordering.
File summaries
| File | Summary |
|---|---|
tests/memory/test_openai_conversations_session.py |
Verifies local limiting and pagination behavior. |
src/agents/memory/openai_conversations_session.py |
Applies limits after provider pagination. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f44c8eb733
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async for item in self._openai_client.conversations.items.list( | ||
| conversation_id=session_id, | ||
| limit=session_limit, | ||
| order="desc", | ||
| ): |
There was a problem hiding this comment.
Bound the provider page size for small limits
When callers use a small positive history limit (for example, get_items(limit=1)) on conversations containing large item payloads, omitting limit makes the paginator fetch its full default page before this loop can break. Previously the request fetched no more than the requested number of items; retain a provider-safe capped page size so oversized limits paginate but the common small-window path does not download unnecessary conversation data and add avoidable latency.
Useful? React with 👍 / 👎.
Summary
OpenAIConversationsSession.get_items(limit=N)was forwarding the local session limit as the Conversations API page size. Provider page-size limits could therefore reject otherwise valid session reads before pagination ran. This change lets the existing async iterator own provider pagination, applies the session cutoff locally, and preserves chronological ordering.Test plan
uv run pytest tests/memory/test_openai_conversations_session.py tests/memory/test_session_limit.py -q— 56 passed (16 existing asyncio deprecation warnings).uv run ruff check src/agents/memory/openai_conversations_session.py tests/memory/test_openai_conversations_session.py— passed.git diff --check— passed.mypyandpyrightchecks for the changed runtime file — passed.Issue number
Closes #4757
Checks
.agents/skills/code-change-verification/scripts/run.sh/reviewbefore submitting this PR