mirror of
https://github.com/openai/codex.git
synced 2026-10-22 17:58:46 +00:00
Track code-mode cell lifecycle and result buffering time (#55653)
## What changed - Add `CodeModeCellTiming` notifications for yielded executions, terminal waits, and unavailable cells, with thread, call, and cell identifiers. - Measure how long completed results remain buffered before retrieval and carry `result_buffered_for` through the runtime, stdio protocol, and gRPC response metadata. Immediate delivery reports zero; waits without a completed result report no buffering measurement. - Default missing buffering data to zero for compatibility with older hosts. ## Testing Extend runtime tests to verify buffering duration with a paused clock and retrieval through termination. Add gRPC conversion tests for nanosecond precision, optional metadata, and the distinction between immediate delivery and no completed result. GitOrigin-RevId: 60c0c5a3affa69ac3779bf1812fca50f5748af76
This commit is contained in:
committed by
copyberry
parent
801bd4d773
commit
87f3c3284e
@@ -117,24 +117,42 @@ pub(super) fn runtime_response(outcome: grpc::ExecutionOutcome) -> Result<Runtim
|
||||
cell_id,
|
||||
content_items,
|
||||
error_text: completed.error_text,
|
||||
result_buffered_for: Duration::ZERO,
|
||||
code_mode_host_duration,
|
||||
},
|
||||
};
|
||||
Ok(response)
|
||||
}
|
||||
|
||||
pub(super) fn wait_outcome(response: grpc::WaitResponse) -> Result<WaitOutcome, String> {
|
||||
match response
|
||||
pub(super) fn wait_outcome(
|
||||
response: tonic::Response<grpc::WaitResponse>,
|
||||
) -> Result<WaitOutcome, String> {
|
||||
let (metadata, response, _) = response.into_parts();
|
||||
let outcome = match response
|
||||
.state
|
||||
.ok_or_else(|| "code-mode wait omitted its outcome".to_string())?
|
||||
{
|
||||
grpc::wait_response::State::LiveCell(response) => {
|
||||
runtime_response(response).map(WaitOutcome::LiveCell)
|
||||
let mut response = runtime_response(response)?;
|
||||
if let RuntimeResponse::Result {
|
||||
result_buffered_for,
|
||||
..
|
||||
} = &mut response
|
||||
{
|
||||
// Older hosts may omit this metadata, including for completed results.
|
||||
*result_buffered_for = metadata
|
||||
.get(grpc::RESULT_BUFFERED_FOR_NS_METADATA)
|
||||
.and_then(|value| value.to_str().ok()?.parse().ok())
|
||||
.map(Duration::from_nanos)
|
||||
.unwrap_or_default();
|
||||
}
|
||||
WaitOutcome::LiveCell(response)
|
||||
}
|
||||
grpc::wait_response::State::MissingCell(response) => {
|
||||
runtime_response(response).map(WaitOutcome::MissingCell)
|
||||
WaitOutcome::MissingCell(runtime_response(response)?)
|
||||
}
|
||||
}
|
||||
};
|
||||
Ok(outcome)
|
||||
}
|
||||
|
||||
fn content_item(item: grpc::ContentItem) -> Result<FunctionCallOutputContentItem, String> {
|
||||
|
||||
@@ -137,6 +137,7 @@ fn runtime_response_decodes_mixed_content_items() {
|
||||
audio_url: "data:audio/wav;base64,AA==".to_string(),
|
||||
},
|
||||
],
|
||||
result_buffered_for: Duration::ZERO,
|
||||
error_text: Some("warning".to_string()),
|
||||
})
|
||||
);
|
||||
@@ -158,7 +159,7 @@ fn wait_outcome_preserves_missing_cell_state() {
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
wait_outcome(response),
|
||||
wait_outcome(tonic::Response::new(response)),
|
||||
Ok(WaitOutcome::MissingCell(RuntimeResponse::Terminated {
|
||||
code_mode_host_duration: Some(Duration::ZERO),
|
||||
cell_id: CellId::new("missing".to_string()),
|
||||
@@ -204,7 +205,7 @@ fn invalid_output_enums_and_missing_oneofs_are_rejected() {
|
||||
};
|
||||
|
||||
assert!(runtime_response(invalid_image).is_err());
|
||||
assert!(wait_outcome(grpc::WaitResponse { state: None }).is_err());
|
||||
assert!(wait_outcome(tonic::Response::new(grpc::WaitResponse { state: None })).is_err());
|
||||
}
|
||||
|
||||
/// Zero is a valid measurement, and decoding must not round nanoseconds to
|
||||
@@ -227,10 +228,38 @@ fn host_timing_preserves_zero_and_nanosecond_precision() {
|
||||
};
|
||||
assert_eq!(runtime_response(outcome.clone()).as_ref(), Ok(&expected));
|
||||
assert_eq!(
|
||||
wait_outcome(grpc::WaitResponse {
|
||||
wait_outcome(tonic::Response::new(grpc::WaitResponse {
|
||||
state: Some(grpc::wait_response::State::LiveCell(outcome)),
|
||||
}),
|
||||
})),
|
||||
Ok(WaitOutcome::LiveCell(expected))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn completed_wait_reads_optional_buffered_duration_from_response_metadata() {
|
||||
for (metadata, expected_ns) in [(None, 0), (Some("0"), 0), (Some("42"), 42)] {
|
||||
let mut response = tonic::Response::new(grpc::WaitResponse {
|
||||
state: Some(grpc::wait_response::State::LiveCell(
|
||||
grpc::ExecutionOutcome {
|
||||
cell_id: "cell".to_string(),
|
||||
outcome: Some(grpc::execution_outcome::Outcome::Completed(
|
||||
grpc::ExecutionCompleted::default(),
|
||||
)),
|
||||
..Default::default()
|
||||
},
|
||||
)),
|
||||
});
|
||||
if let Some(value) = metadata {
|
||||
response.metadata_mut().insert(
|
||||
grpc::RESULT_BUFFERED_FOR_NS_METADATA,
|
||||
value.parse().unwrap(),
|
||||
);
|
||||
}
|
||||
let wait = wait_outcome(response).unwrap();
|
||||
assert_eq!(
|
||||
wait.result_buffered_for(),
|
||||
Some(Duration::from_nanos(expected_ns))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -103,11 +103,13 @@ fn public_runtime_response(generation: u64, response: RuntimeResponse) -> Runtim
|
||||
RuntimeResponse::Result {
|
||||
cell_id,
|
||||
content_items,
|
||||
result_buffered_for,
|
||||
error_text,
|
||||
code_mode_host_duration,
|
||||
} => RuntimeResponse::Result {
|
||||
cell_id: public_cell_id(generation, &cell_id),
|
||||
content_items,
|
||||
result_buffered_for,
|
||||
error_text,
|
||||
code_mode_host_duration,
|
||||
},
|
||||
|
||||
@@ -177,6 +177,7 @@ async fn reconnected_execution_maps_started_and_initial_response_ids() {
|
||||
content_items: vec![FunctionCallOutputContentItem::InputText {
|
||||
text: "result".to_string(),
|
||||
}],
|
||||
result_buffered_for: Duration::ZERO,
|
||||
error_text: None,
|
||||
};
|
||||
let started = StartedCell::from_future(wire_id, async move {
|
||||
@@ -199,6 +200,7 @@ async fn reconnected_execution_maps_started_and_initial_response_ids() {
|
||||
content_items: vec![FunctionCallOutputContentItem::InputText {
|
||||
text: "result".to_string(),
|
||||
}],
|
||||
result_buffered_for: Duration::ZERO,
|
||||
error_text: None,
|
||||
})
|
||||
);
|
||||
|
||||
@@ -331,7 +331,7 @@ impl SessionInner {
|
||||
drop(watcher);
|
||||
cancellation.disarm();
|
||||
self.prune_wait_slots();
|
||||
let outcome = conversion::wait_outcome(response?.into_inner())?;
|
||||
let outcome = conversion::wait_outcome(response?)?;
|
||||
self.validate_wait_cell(&expected_cell_id, outcome).await
|
||||
}
|
||||
|
||||
@@ -370,8 +370,7 @@ impl SessionInner {
|
||||
cell_id: cell_id.as_str().to_string(),
|
||||
})),
|
||||
)
|
||||
.await?
|
||||
.into_inner();
|
||||
.await?;
|
||||
let outcome = conversion::wait_outcome(response)?;
|
||||
self.validate_wait_cell(&cell_id, outcome).await
|
||||
}
|
||||
|
||||
@@ -78,11 +78,13 @@ pub(super) fn public_runtime_response(
|
||||
RuntimeResponse::Result {
|
||||
cell_id,
|
||||
content_items,
|
||||
result_buffered_for,
|
||||
error_text,
|
||||
code_mode_host_duration,
|
||||
} => RuntimeResponse::Result {
|
||||
cell_id: public_cell_id_from_protocol(generation, &cell_id),
|
||||
content_items,
|
||||
result_buffered_for,
|
||||
error_text,
|
||||
code_mode_host_duration,
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user