[codex] Flush trailing realtime transcript tail - #29918
Conversation
ac63f06 to
31cdb43
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31cdb43e0d
ℹ️ 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".
| if flush_transcript_tail_on_session_end | ||
| && let Some(transcript_delta) = | ||
| realtime_transcript_delta(&events.take_transcript_tail().await) |
There was a problem hiding this comment.
When an opted-in realtime session accumulates a long tail after the last handoff, this converts the entire take_transcript_tail() result into a synthetic user turn with no token or byte cap before it enters model context. A multi-minute call can easily exceed 1k tokens or grow unbounded, causing context/cache blowups on session close; please truncate or otherwise hard-cap the tail before routing it.
AGENTS.md reference: AGENTS.md:L91-L100
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I think we should not truncate here. It's crucial that the raw transcripts are not lost -- thats the whole point of the PR
| // Drain already-parsed events so a queued handoff is routed before the final tail. | ||
| while let Ok(event) = events_rx.recv().await { |
There was a problem hiding this comment.
When flushTranscriptTailOnSessionEnd is omitted or false, thread/realtime/stop still now waits for this drain and routes any already-queued HandoffRequested via route_realtime_text_input, so a client that stops a realtime session can unexpectedly start a new Codex turn from an old handoff that previously would have been aborted. Please gate this post-stop handoff routing to the tail-flush opt-in path or skip it once shutdown was requested.
AGENTS.md reference: AGENTS.md:L102-L110
Useful? React with 👍 / 👎.
| pub client_managed_handoffs: Option<bool>, | ||
| /// Routes any transcript tail remaining at session end through Codex. Defaults to false. | ||
| #[ts(optional = nullable)] | ||
| pub flush_transcript_tail_on_session_end: Option<bool>, |
There was a problem hiding this comment.
This adds a client-visible thread/realtime/start parameter, but the app-server API docs for that method still do not mention flushTranscriptTailOnSessionEnd, so experimental clients cannot discover the opt-in behavior or its default from the documented API surface. Please update app-server/README.md alongside this protocol change.
AGENTS.md reference: AGENTS.md:L298-L303
Useful? React with 👍 / 👎.
| pub client_managed_handoffs: Option<bool>, | ||
| /// Routes any transcript tail remaining at session end through Codex. Defaults to false. | ||
| #[ts(optional = nullable)] | ||
| pub flush_transcript_tail_on_session_end: Option<bool>, |
There was a problem hiding this comment.
This new client-to-server v2 boolean is interpreted with unwrap_or(false), so omission means false; modeling it as Option<bool> instead publishes a nullable field and serializes None as null in generated request shapes. Please use the defaulted bool shape for false-default request flags so the wire/API contract matches the v2 convention.
AGENTS.md reference: AGENTS.md:L289-L294
Useful? React with 👍 / 👎.
31cdb43 to
3c67d5f
Compare
Why
A realtime session can end after more transcript has accumulated than was included in its last handoff. That tail already lives in core's active transcript state, but the stop path aborted the realtime input/fanout tasks before routing it, so the final bit of the conversation could disappear before
thread/realtime/closed.This behavior is still being evaluated, so clients must opt in per realtime session. Omitted or false leaves shutdown behavior unchanged.
What changed
flushTranscriptTailOnSessionEndtothread/realtime/start, defaulting to false in app-server.take_transcript_tail()from the existing active transcript state usinglast_handoff_entry_count.<realtime_delegation>with the remaining text in<transcript_delta>.Validation
just test -p codex-app-server-protocoljust test -p codex-app-server realtime_conversation_stop_emits_closed_notificationjust test -p codex-core conversation_transport_close_just test -p codex-core conversation_close_routes_only_remaining_transcript_tail_oncejust fixforcodex-app-server-protocol,codex-app-server,codex-protocol, andcodex-corejust fmt