mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
linearize history output normalization (#28309)
## Why When we prepare the conversation history, every tool call needs a matching output. Before this change, we scanned the full history again for every call. In a tool-heavy conversation, that makes the work `O(items x calls)`, or `O(n^2)` in the worst case. ## What Scan the history once and collect the IDs of existing outputs. Then each call can check its ID with an expected `O(1)` lookup. The full normalization step is now expected `O(n)`. The output order and missing-output behavior stay the same. ## Performance Based on local rollout traces, one tool-heavy session reached roughly 17,050 transcript items with about 4,292 tool-call items. On a history of that shape, the old `calls x items` scan does about 73.2 million membership checks, while the new pass does about 21.3 thousand set inserts/lookups. That is roughly 3.4k times less membership work in this normalization step. ## Validation - `just test -p codex-core normalize_` (19 passed)
This commit is contained in:
@@ -12,6 +12,27 @@ const IMAGE_CONTENT_OMITTED_PLACEHOLDER: &str =
|
||||
"image content omitted because you do not support image input";
|
||||
|
||||
pub(crate) fn ensure_call_outputs_present(items: &mut Vec<ResponseItem>) {
|
||||
let mut function_output_ids = HashSet::new();
|
||||
let mut tool_search_output_ids = HashSet::new();
|
||||
let mut custom_tool_output_ids = HashSet::new();
|
||||
for item in items.iter() {
|
||||
match item {
|
||||
ResponseItem::FunctionCallOutput { call_id, .. } => {
|
||||
function_output_ids.insert(call_id.as_str());
|
||||
}
|
||||
ResponseItem::ToolSearchOutput {
|
||||
call_id: Some(call_id),
|
||||
..
|
||||
} => {
|
||||
tool_search_output_ids.insert(call_id.as_str());
|
||||
}
|
||||
ResponseItem::CustomToolCallOutput { call_id, .. } => {
|
||||
custom_tool_output_ids.insert(call_id.as_str());
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
// Collect synthetic outputs to insert immediately after their calls.
|
||||
// Store the insertion position (index of call) alongside the item so
|
||||
// we can insert in reverse order and avoid index shifting.
|
||||
@@ -19,99 +40,72 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec<ResponseItem>) {
|
||||
|
||||
for (idx, item) in items.iter().enumerate() {
|
||||
match item {
|
||||
ResponseItem::FunctionCall { call_id, .. } => {
|
||||
let has_output = items.iter().any(|i| match i {
|
||||
ResponseItem::FunctionCall { call_id, .. }
|
||||
if !function_output_ids.contains(call_id.as_str()) =>
|
||||
{
|
||||
info!("Function call output is missing for call id: {call_id}");
|
||||
missing_outputs_to_insert.push((
|
||||
idx,
|
||||
ResponseItem::FunctionCallOutput {
|
||||
call_id: existing, ..
|
||||
} => existing == call_id,
|
||||
_ => false,
|
||||
});
|
||||
|
||||
if !has_output {
|
||||
info!("Function call output is missing for call id: {call_id}");
|
||||
missing_outputs_to_insert.push((
|
||||
idx,
|
||||
ResponseItem::FunctionCallOutput {
|
||||
call_id: call_id.clone(),
|
||||
output: FunctionCallOutputPayload::from_text("aborted".to_string()),
|
||||
},
|
||||
));
|
||||
}
|
||||
call_id: call_id.clone(),
|
||||
output: FunctionCallOutputPayload::from_text("aborted".to_string()),
|
||||
},
|
||||
));
|
||||
}
|
||||
ResponseItem::ToolSearchCall {
|
||||
call_id: Some(call_id),
|
||||
..
|
||||
} => {
|
||||
let has_output = items.iter().any(|i| match i {
|
||||
} if !tool_search_output_ids.contains(call_id.as_str()) => {
|
||||
info!("Tool search output is missing for call id: {call_id}");
|
||||
missing_outputs_to_insert.push((
|
||||
idx,
|
||||
ResponseItem::ToolSearchOutput {
|
||||
call_id: Some(existing),
|
||||
..
|
||||
} => existing == call_id,
|
||||
_ => false,
|
||||
});
|
||||
|
||||
if !has_output {
|
||||
info!("Tool search output is missing for call id: {call_id}");
|
||||
missing_outputs_to_insert.push((
|
||||
idx,
|
||||
ResponseItem::ToolSearchOutput {
|
||||
call_id: Some(call_id.clone()),
|
||||
status: "completed".to_string(),
|
||||
execution: "client".to_string(),
|
||||
tools: Vec::new(),
|
||||
},
|
||||
));
|
||||
}
|
||||
call_id: Some(call_id.clone()),
|
||||
status: "completed".to_string(),
|
||||
execution: "client".to_string(),
|
||||
tools: Vec::new(),
|
||||
},
|
||||
));
|
||||
}
|
||||
ResponseItem::CustomToolCall { call_id, .. } => {
|
||||
let has_output = items.iter().any(|i| match i {
|
||||
ResponseItem::CustomToolCall { call_id, .. }
|
||||
if !custom_tool_output_ids.contains(call_id.as_str()) =>
|
||||
{
|
||||
error_or_panic(format!(
|
||||
"Custom tool call output is missing for call id: {call_id}"
|
||||
));
|
||||
missing_outputs_to_insert.push((
|
||||
idx,
|
||||
ResponseItem::CustomToolCallOutput {
|
||||
call_id: existing, ..
|
||||
} => existing == call_id,
|
||||
_ => false,
|
||||
});
|
||||
|
||||
if !has_output {
|
||||
error_or_panic(format!(
|
||||
"Custom tool call output is missing for call id: {call_id}"
|
||||
));
|
||||
missing_outputs_to_insert.push((
|
||||
idx,
|
||||
ResponseItem::CustomToolCallOutput {
|
||||
call_id: call_id.clone(),
|
||||
name: None,
|
||||
output: FunctionCallOutputPayload::from_text("aborted".to_string()),
|
||||
},
|
||||
));
|
||||
}
|
||||
call_id: call_id.clone(),
|
||||
name: None,
|
||||
output: FunctionCallOutputPayload::from_text("aborted".to_string()),
|
||||
},
|
||||
));
|
||||
}
|
||||
// LocalShellCall is represented in upstream streams by a FunctionCallOutput
|
||||
ResponseItem::LocalShellCall { call_id, .. } => {
|
||||
if let Some(call_id) = call_id.as_ref() {
|
||||
let has_output = items.iter().any(|i| match i {
|
||||
ResponseItem::FunctionCallOutput {
|
||||
call_id: existing, ..
|
||||
} => existing == call_id,
|
||||
_ => false,
|
||||
});
|
||||
|
||||
if !has_output {
|
||||
error_or_panic(format!(
|
||||
"Local shell call output is missing for call id: {call_id}"
|
||||
));
|
||||
missing_outputs_to_insert.push((
|
||||
idx,
|
||||
ResponseItem::FunctionCallOutput {
|
||||
call_id: call_id.clone(),
|
||||
output: FunctionCallOutputPayload::from_text("aborted".to_string()),
|
||||
},
|
||||
));
|
||||
}
|
||||
}
|
||||
ResponseItem::LocalShellCall {
|
||||
call_id: Some(call_id),
|
||||
..
|
||||
} if !function_output_ids.contains(call_id.as_str()) => {
|
||||
error_or_panic(format!(
|
||||
"Local shell call output is missing for call id: {call_id}"
|
||||
));
|
||||
missing_outputs_to_insert.push((
|
||||
idx,
|
||||
ResponseItem::FunctionCallOutput {
|
||||
call_id: call_id.clone(),
|
||||
output: FunctionCallOutputPayload::from_text("aborted".to_string()),
|
||||
},
|
||||
));
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
drop((
|
||||
function_output_ids,
|
||||
tool_search_output_ids,
|
||||
custom_tool_output_ids,
|
||||
));
|
||||
|
||||
// Insert synthetic outputs in reverse index order to avoid re-indexing.
|
||||
for (idx, output_item) in missing_outputs_to_insert.into_iter().rev() {
|
||||
|
||||
Reference in New Issue
Block a user