fix(sessions): chunk Conversations item creates at the API batch limit - #4649
fix(sessions): chunk Conversations item creates at the API batch limit#4649hsusul wants to merge 2 commits into
Conversation
OpenAIConversationsSession.add_items sent the full list in one conversations.items.create call. That endpoint accepts at most 20 items, so a tool-heavy turn (or any batch of 21+) failed with HTTP 400. Split creates into 20-item batches and delete earlier chunks if a later create fails so the batch stays all-or-nothing.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4af64b616
ℹ️ 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".
| items=items[offset : offset + _MAX_ITEMS_PER_CONVERSATION_CREATE], | ||
| ) | ||
| created_ids.extend(_created_conversation_item_ids(created)) | ||
| except Exception: |
There was a problem hiding this comment.
When the task is cancelled after at least one 20-item request succeeds—for example, while awaiting the next request—asyncio.CancelledError inherits from BaseException, so this except Exception block never runs. The completed prefix remains in the remote conversation even though add_items() raises cancellation, exposing a partial logical batch to the next run; include cancellation in the rollback path and protect cleanup while re-raising it.
AGENTS.md reference: AGENTS.md:L149-L150
Useful? React with 👍 / 👎.
seratch
changed the title
seratch
left a comment
seratch
left a comment
There was a problem hiding this comment.
Before merge, please make the multi-request append preserve the existing logical-batch semantics under cancellation and same-session concurrency.
asyncio.CancelledError bypasses the current except Exception, so cancellation after one chunk commits leaves a partial batch. Protect and await rollback before re-raising cancellation, and serialize the complete append/rollback phase against get_items(), pop_item(), clear_session(), and other add_items() calls on the same session instance. Please add controlled tests for cancellation and an overlapping surviving operation, asserting both operation results and final history.
CancelledError skipped the Exception rollback path, so a cancel after the first 20-item create left a partial batch. Roll back committed chunks before re-raising cancellation, serialize the full append/rollback against other same-session operations, and test overlapping survivors.
|
Addressed the cancellation and same-session concurrency gaps.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3da594e96f
ℹ️ 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 def _delete_created_items(self, session_id: str, created_ids: list[str]) -> None: | ||
| for item_id in reversed(created_ids): | ||
| with contextlib.suppress(Exception): |
There was a problem hiding this comment.
When a rollback delete receives a persistent API error, such as a rate-limit or server error after client retries, this suppression lets add_items() re-raise only the later create failure while leaving that earlier chunk item in the conversation. The failed logical batch then remains partially visible and a caller retry can duplicate it without any indication that cleanup was incomplete; preserve the primary create failure while making incomplete rollback observable or otherwise preventing the inconsistent session from being reused.
AGENTS.md reference: AGENTS.md:L149-L150
Useful? React with 👍 / 👎.
2 participants
Summary
OpenAIConversationsSession.add_items()forwarded the entire list toconversations.items.create. The Conversations API allows at most 20 items per create, so a Runner turn with many parallel tool calls (11 tools → 11 calls + 11 outputs = 22 items) fails with HTTP 400. SQLite/Redis sessions have no such cap, so the same session workflow works there and breaks here.Reproduction
Solution
add_itemsinto sequential creates of at most 20 items, preserving order.Test plan
uv run pytest tests/memory/test_openai_conversations_session.py tests/memory/test_session_limit.py(56 passed)ruff/pyrighton the changed filesmake typecheckpassedIssue number
N/A. Related but different: closed #4228 was about
items.listpage size 1–100, not create batch size 20.Checks
/reviewbefore submitting this PR