fix(realtime): preserve item status on retrieved conversation items - #4598
fix(realtime): preserve item status on retrieved conversation items#4598shoutoutuoadi325 wants to merge 3 commits into
Conversation
leadingproblemsolver
left a comment
There was a problem hiding this comment.
The converter fix looks right, but I think the regression test still stops one boundary too early. test_retrieved_completed_item_keeps_status asserts the emitted RealtimeModelItemUpdatedEvent; it never feeds that event through RealtimeSession, which is where the existing history item is actually reconciled/replaced.
That means this test could stay green while a later merge-path regression still turns a known terminal item into in_progress or drops content evidence.
I’d add one session-level fixture with an existing assistant history item (status="completed", transcript present), then process a retrieved update and assert the final session._history item preserves both lifecycle state and transcript. Parameterizing completed / incomplete would cover both terminal statuses.
There is one subtle edge worth making intentional too: item.status or "in_progress" is safe for create-like events with no status, but ConversationItem.status is optional. If a retrieved item ever arrives with status=None, this fallback can still regress an already-known terminal history item. Either assert the protocol guarantees status on retrieve, or make the session reconciliation monotonic when incoming status is absent.
Stronger contract: a refresh may add information, but it should not erase already-known terminal lifecycle state unless the protocol explicitly reports a new state.
seratch
left a comment
There was a problem hiding this comment.
Thanks for the focused fix. The production change correctly preserves an explicit server status while retaining the released missing-status fallback.
Before merge, please extend the regression coverage through RealtimeSession: attach a session listener, seed its history with a completed assistant item containing a transcript, feed a conversation.item.retrieved event that reports completed while omitting the transcript, and assert that the final history item retains both status="completed" and the existing transcript. The current test stops at RealtimeModelItemUpdatedEvent, so it does not lock down the user-visible history outcome reported in #4597.
Please keep the current item.status or "in_progress" behavior. Changing omitted-status semantics or adding a new monotonic session-state rule would widen this fix beyond the demonstrated case.
|
Thanks @seratch! I've added the requested session-level coverage in I also verified the new test fails against the pre-fix converter ( |
Summary
conversation_item_to_realtime_message_itemhardcodedstatus: "in_progress", discarding the status carried by the server item. When the SDK retrieves the last assistant audio item afterconversation.item.input_audio_transcription.completed(orconversation.item.truncated), the server repliesconversation.item.retrievedwith the item's real status, usuallycompleted. The conversion rewrote that toin_progress, and the session history merge keeps the incoming status, so a completed history entry regressed toin_progressafter every user turn, with nothing to restore it.The conversion now passes
item.statusthrough and falls back toin_progresswhen the server omits it, so streamingconversation.item.createdevents that carry no status behave exactly as before.Test plan
uv run pytest tests/realtime/test_item_parsing.py(status preserved; fallback toin_progresswhen absent)uv run pytest tests/realtime/test_openai_realtime.py::TestEventHandlingRobustness::test_retrieved_completed_item_keeps_status(audio delta → transcription completed triggers the retrieve → retrieved item keepscompleted)mainand pass with the fixuv run pytest tests/realtime/(518 passed).agents/skills/code-change-verification/scripts/run.sh(format, lint, typecheck, full test suite)Issue number
Fixes #4597
Checks
.agents/skills/code-change-verification/scripts/run.sh/reviewbefore submitting this PR