Skip to content

fix(session_persistence): count persisted items after filtering unpersistable conversation items #3580

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mshsheikh wants to merge 4 commits into
base: main
Choose a base branch
from
+20 −6
Open

fix(session_persistence): count persisted items after filtering unpersistable conversation items #3580

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
51e42ef
fix(session_persistence): count persisted items after filtering unper…
mshsheikh Jun 4, 2026
cc47d8d
fix(session_persistence): keep raw retry offset aligned with persiste…
mshsheikh Jun 4, 2026
3e20b7e
fix(session_persistence): use raw retry offset for conversation turn …
mshsheikh Jun 4, 2026
5f050e4
fix(session_persistence): return raw offset for resumed OpenAI conver…
mshsheikh Jun 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions src/agents/run_internal/session_persistence.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,10 @@ async def save_result_to_session(
if missing_outputs:
new_run_items = missing_outputs + new_run_items

# Raw retry offset: count the run items we consumed from this turn,
# not just the subset that was actually persisted.
new_run_items_raw_count = len(new_run_items)

input_list: list[TResponseInputItem] = []
if original_input:
input_list = normalize_input_items_for_api(
Expand Down Expand Up @@ -322,6 +326,11 @@ async def save_result_to_session(
if is_openai_conversation_session and items_to_save:
items_to_save = [_sanitize_openai_conversation_item(item) for item in items_to_save]

if is_openai_conversation_session:
items_to_save = [
item for item in items_to_save if not _is_unpersistable_for_openai_conversation(item)
]
Comment on lines +330 to +332
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 4, 2026

Choose a reason for hiding this comment

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

P2 Badge Keep retry offset aligned with raw run items

For OpenAIConversationsSession turns where an unpersistable reasoning item precedes a persistable item, this filtering makes saved_run_items_count exclude the reasoning item even though _current_turn_persisted_item_count is later used as a slice index into the original, unfiltered new_items list (new_items[already_persisted:]). For example, after saving [bad reasoning, assistant message], the counter becomes 1; a retry/resume starts at raw index 1 and saves the already-persisted assistant message again, causing duplicate session items. The accounting needs either a separate raw-item offset or filtering before the slice/counting logic so retries skip the same items that were already considered.

Useful? React with 👍 / 👎.

Comment on lines +329 to +332
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 4, 2026

Choose a reason for hiding this comment

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

P2 Badge Return the raw consumed count for resumed saves

In the resumed-run path I checked (src/agents/run.py lines 1339-1351), this helper is called with run_state=None and the caller advances _current_turn_persisted_item_count by the returned value. After filtering unpersistable OpenAI Conversations reasoning items here, a resumed turn like [reasoning without id/encrypted_content, assistant message] returns 1 even though two raw run items were consumed, so the next retry slices at the already-saved assistant message and persists it again. The in-function state update now uses the raw count, but the return value still needs the same raw-offset semantics for callers that update state themselves.

Useful? React with 👍 / 👎.


serialized_to_save: list[str] = [
_fingerprint_or_repr(item, ignore_ids_for_matching=ignore_ids_for_matching)
for item in items_to_save
Expand All @@ -336,20 +345,25 @@ async def save_result_to_session(
serialized_to_save_counts[serialized] -= 1
saved_run_items_count += 1

if is_openai_conversation_session:
items_to_save = [
item for item in items_to_save if not _is_unpersistable_for_openai_conversation(item)
]
returned_count = (
new_run_items_raw_count
if is_openai_conversation_session and run_state is None
else saved_run_items_count
)

if len(items_to_save) == 0:
if run_state:
run_state._current_turn_persisted_item_count = already_persisted + saved_run_items_count
run_state._current_turn_persisted_item_count = (
already_persisted + new_run_items_raw_count
)
Comment on lines +356 to +358
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 4, 2026

Choose a reason for hiding this comment

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

P2 Badge Do not advance ordinary sessions for unsaved items

For non-OpenAI sessions, a ToolApprovalItem is skipped by run_item_to_input_item, so items_to_save can be empty even though new_run_items_raw_count is nonzero. This now records the raw item as persisted, leaving _current_turn_persisted_item_count > 0 after nothing was written; later same-turn persistence paths use that counter to skip or slice items, so a retry/resume after an approval-only interruption can miss subsequently persistable output. The raw-offset accounting should be limited to the OpenAI Conversations filtering case, while ordinary sessions keep the actual saved count.

Useful? React with 👍 / 👎.

return saved_run_items_count

await session.add_items(items_to_save)

if run_state:
run_state._current_turn_persisted_item_count = already_persisted + saved_run_items_count
run_state._current_turn_persisted_item_count = (
already_persisted + new_run_items_raw_count
)
Comment on lines +364 to +366
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 4, 2026

Choose a reason for hiding this comment

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

P2 Badge Use the raw item offset when updating retry state

When an OpenAI Conversations turn has an unpersistable reasoning item before a persistable item, saved_run_items_count now excludes the reasoning item after filtering, but _current_turn_persisted_item_count is still used as a raw slice index into new_items on the next call; a retry/resume then starts at the already-saved persistable item and writes it again. The fresh evidence in this revision is that new_run_items_raw_count was added to represent the consumed raw offset but is never used for this state update.

Useful? React with 👍 / 👎.


if response_id and is_openai_responses_compaction_aware_session(session):
has_local_tool_outputs = any(
Expand Down