mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
Handle partial answers in realtime routing and thread search (#51260)
## Why Messages with the `partial_answer` phase contain answer text without declaring a terminal answer. They should remain searchable and speakable without being treated as final messages. ## What changed - Preserve explicit `partial_answer` phases in streamed and completed realtime handoffs, bypassing legacy channel-prefix parsing and routing them to the `speakable` channel. - Treat partial answers as nonterminal in V1 handoffs and omit the final-message prefix. - Include partial answers in thread occurrence searches and normalize them to the final conversation channel in rollout traces. ## Testing Extend coverage for realtime phase routing, streamed and completed handoffs, partial-answer search results, and rollout normalization. Verify that turn summaries still exclude nonterminal messages. GitOrigin-RevId: aa183da783f6e7e0b78ddd51073cba533297e121
This commit is contained in:
@@ -243,7 +243,7 @@ impl RealtimeStreamedItem {
|
||||
fn output_prefix(&self) -> &'static str {
|
||||
if self.prefix_final_message
|
||||
&& self.sent_bytes == 0
|
||||
&& !matches!(self.phase, Some(MessagePhase::Commentary))
|
||||
&& matches!(self.phase, Some(MessagePhase::FinalAnswer) | None)
|
||||
{
|
||||
AGENT_FINAL_MESSAGE_PREFIX
|
||||
} else {
|
||||
@@ -928,9 +928,9 @@ impl RealtimeConversationManager {
|
||||
if handoff.client_managed_handoffs {
|
||||
return Ok(());
|
||||
}
|
||||
// TODO: Preserve explicit PartialAnswer phases through completed and streamed BEM
|
||||
// routing, with integration coverage asserting the speakable channel.
|
||||
let phase = if handoff.routes_handoff_by_bem() {
|
||||
let phase = if handoff.routes_handoff_by_bem()
|
||||
&& !matches!(phase, Some(MessagePhase::PartialAnswer))
|
||||
{
|
||||
match bem_message_phase(
|
||||
&output_text,
|
||||
&handoff.codex_response_handoff_channel_prefixes,
|
||||
@@ -944,9 +944,10 @@ impl RealtimeConversationManager {
|
||||
} else {
|
||||
phase
|
||||
};
|
||||
// TODO: Treat PartialAnswer as nonterminal for V1 handoffs, omitting the final-message
|
||||
// prefix for streamed, completed-item, and standalone output.
|
||||
let is_commentary = matches!(phase, Some(MessagePhase::Commentary));
|
||||
let is_nonterminal = matches!(
|
||||
phase,
|
||||
Some(MessagePhase::Commentary | MessagePhase::PartialAnswer)
|
||||
);
|
||||
let active_handoff = handoff.stream.lock().await.active_handoff.clone();
|
||||
let output = match active_handoff {
|
||||
Some(handoff_id) => {
|
||||
@@ -963,7 +964,7 @@ impl RealtimeConversationManager {
|
||||
),
|
||||
phase,
|
||||
}
|
||||
} else if handoff.event_parser == RealtimeEventParser::V1 && is_commentary {
|
||||
} else if handoff.event_parser == RealtimeEventParser::V1 && is_nonterminal {
|
||||
RealtimeOutbound::HandoffAppend {
|
||||
handoff_id,
|
||||
text: output_text,
|
||||
@@ -990,7 +991,8 @@ impl RealtimeConversationManager {
|
||||
}
|
||||
} else {
|
||||
RealtimeOutbound::StandaloneHandoff {
|
||||
text: if handoff.event_parser == RealtimeEventParser::V1 && !is_commentary {
|
||||
text: if handoff.event_parser == RealtimeEventParser::V1 && !is_nonterminal
|
||||
{
|
||||
format!("{AGENT_FINAL_MESSAGE_PREFIX}{output_text}")
|
||||
} else {
|
||||
output_text
|
||||
@@ -1032,14 +1034,14 @@ impl RealtimeConversationManager {
|
||||
let Some(handoff_id) = stream.active_handoff.clone() else {
|
||||
return;
|
||||
};
|
||||
// An explicit stable fragment is answer text, even when it starts
|
||||
// with characters that legacy BEM treats as a private channel tag.
|
||||
let parse_bem = handoff.routes_handoff_by_bem()
|
||||
&& !matches!(phase, Some(MessagePhase::PartialAnswer));
|
||||
let mut streamed_item = RealtimeStreamedItem {
|
||||
handoff_id,
|
||||
phase: if handoff.routes_handoff_by_bem() {
|
||||
None
|
||||
} else {
|
||||
phase
|
||||
},
|
||||
bem_channel_parser: handoff.routes_handoff_by_bem().then(|| {
|
||||
phase: if parse_bem { None } else { phase },
|
||||
bem_channel_parser: parse_bem.then(|| {
|
||||
BemChannelParser::new(Arc::clone(
|
||||
&handoff.codex_response_handoff_channel_prefixes,
|
||||
))
|
||||
|
||||
@@ -26,6 +26,7 @@ use pretty_assertions::assert_eq;
|
||||
use std::collections::BTreeMap;
|
||||
use std::sync::Arc;
|
||||
use std::time::Instant;
|
||||
use test_case::test_case;
|
||||
use tokio::sync::Mutex;
|
||||
|
||||
#[test]
|
||||
@@ -231,11 +232,17 @@ async fn clears_active_handoff_explicitly() {
|
||||
assert_eq!(state.stream.lock().await.active_handoff.clone(), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn streamed_handoff_preserves_a_bounded_final_tail() {
|
||||
#[test_case(None, true; "legacy")]
|
||||
#[test_case(Some(MessagePhase::FinalAnswer), true; "final_answer")]
|
||||
#[test_case(Some(MessagePhase::Commentary), false; "commentary")]
|
||||
#[test_case(Some(MessagePhase::PartialAnswer), false; "partial_answer")]
|
||||
fn streamed_handoff_preserves_a_bounded_final_tail(
|
||||
phase: Option<MessagePhase>,
|
||||
expect_final_prefix: bool,
|
||||
) {
|
||||
let mut item = RealtimeStreamedItem {
|
||||
handoff_id: "handoff_1".to_string(),
|
||||
phase: Some(MessagePhase::FinalAnswer),
|
||||
phase,
|
||||
bem_channel_parser: None,
|
||||
prefix_final_message: true,
|
||||
sent_bytes: 0,
|
||||
@@ -256,7 +263,12 @@ fn streamed_handoff_preserves_a_bounded_final_tail() {
|
||||
let output = format!("{first}{final_chunk}");
|
||||
|
||||
assert!(output.len() <= 4_000);
|
||||
assert!(output.starts_with(&format!("{AGENT_FINAL_MESSAGE_PREFIX}HEAD")));
|
||||
let prefix = if expect_final_prefix {
|
||||
AGENT_FINAL_MESSAGE_PREFIX
|
||||
} else {
|
||||
""
|
||||
};
|
||||
assert!(output.starts_with(&format!("{prefix}HEAD")));
|
||||
assert!(output.contains(HANDOFF_STREAM_TRUNCATION_MARKER));
|
||||
assert!(output.ends_with("TAIL"));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user