This commit is contained in:
jimmyfraiture
2025-10-01 17:12:29 +01:00
parent a6d3e2a334
commit ad29d2bf39
7 changed files with 36 additions and 136 deletions

View File

@@ -118,11 +118,7 @@ impl ModelClient {
})
}
pub fn supports_parallel_read_only_tools(&self) -> bool {
self.config.enable_parallel_read_only_tools
&& self.config.model_family.supports_parallel_read_only_tools
&& self.provider.supports_parallel_tool_calls
}
// Parallel read-only scheduling is controlled internally; no provider hint needed.
/// Dispatches to either the Responses or Chat implementation depending on
/// the provider config. Public callers always invoke `stream()` the
@@ -232,8 +228,6 @@ impl ModelClient {
input: &input_with_instructions,
tools: &tools_json,
tool_choice: "auto",
parallel_tool_calls: prompt.allow_parallel_read_only_tools
&& self.supports_parallel_read_only_tools(),
reasoning,
store: azure_workaround,
stream: true,

View File

@@ -36,9 +36,6 @@ pub struct Prompt {
/// Optional the output schema for the model's response.
pub output_schema: Option<Value>,
/// Allow parallel tool calls for read-only tools.
pub allow_parallel_read_only_tools: bool,
}
impl Prompt {
@@ -152,7 +149,6 @@ pub(crate) struct ResponsesApiRequest<'a> {
pub(crate) input: &'a Vec<ResponseItem>,
pub(crate) tools: &'a [serde_json::Value],
pub(crate) tool_choice: &'static str,
pub(crate) parallel_tool_calls: bool,
pub(crate) reasoning: Option<Reasoning>,
pub(crate) store: bool,
pub(crate) stream: bool,
@@ -282,7 +278,6 @@ mod tests {
input: &input,
tools: &tools,
tool_choice: "auto",
parallel_tool_calls: false,
reasoning: None,
store: false,
stream: true,
@@ -323,7 +318,6 @@ mod tests {
input: &input,
tools: &tools,
tool_choice: "auto",
parallel_tool_calls: false,
reasoning: None,
store: false,
stream: true,
@@ -359,7 +353,6 @@ mod tests {
input: &input,
tools: &tools,
tool_choice: "auto",
parallel_tool_calls: false,
reasoning: None,
store: false,
stream: true,
@@ -372,40 +365,5 @@ mod tests {
assert!(v.get("text").is_none());
}
#[test]
fn serializes_parallel_tool_calls_flag() {
let input: Vec<ResponseItem> = vec![];
let tools: Vec<serde_json::Value> = vec![];
let req_true = ResponsesApiRequest {
model: "gpt-5",
instructions: "i",
input: &input,
tools: &tools,
tool_choice: "auto",
parallel_tool_calls: true,
reasoning: None,
store: false,
stream: true,
include: vec![],
prompt_cache_key: None,
text: None,
};
let v_true = serde_json::to_value(&req_true).expect("json");
assert_eq!(
v_true.get("parallel_tool_calls"),
Some(&serde_json::Value::Bool(true))
);
let req_false = ResponsesApiRequest {
parallel_tool_calls: false,
..req_true
};
let v_false = serde_json::to_value(&req_false).expect("json");
assert_eq!(
v_false.get("parallel_tool_calls"),
Some(&serde_json::Value::Bool(false))
);
}
// parallel_tool_calls flag removed: scheduling is internal.
}

View File

@@ -1912,18 +1912,13 @@ async fn run_turn(
Some(mcp_tools),
));
let template_executor =
ToolCallExecutor::new(router.clone(), sess.clone(), turn_context.clone());
let allow_parallel_read_only = template_executor.allow_parallel_read_only();
let tool_specs = template_executor.specs().to_vec();
drop(template_executor);
let tool_specs = router.specs().to_vec();
let prompt = Prompt {
input,
tools: tool_specs,
base_instructions_override: turn_context.base_instructions.clone(),
output_schema: turn_context.final_output_json_schema.clone(),
allow_parallel_read_only_tools: allow_parallel_read_only,
};
let mut retries = 0;

View File

@@ -1754,7 +1754,7 @@ model_verbosity = "high"
let mut model_provider_map = built_in_model_providers();
model_provider_map.insert(
"openai-chat-completions".to_string(),
openai_chat_completions_provider.clone(),
openai_chat_completions_provider,
);
model_provider_map
};

View File

@@ -1,4 +1,3 @@
use std::collections::HashMap;
use std::panic::AssertUnwindSafe;
use std::sync::Arc;
@@ -19,7 +18,6 @@ use crate::protocol::Event;
use crate::tools::context::ToolPayload;
use crate::tools::router::Router;
use crate::tools::router::ToolCall;
use crate::tools::spec::ToolSpec;
use crate::turn_diff_tracker::TurnDiffTracker;
use codex_protocol::models::FunctionCallOutputPayload;
use codex_protocol::models::ResponseInputItem;
@@ -36,20 +34,18 @@ pub(crate) struct ToolCallExecutor {
session: Arc<Session>,
turn_context: Arc<TurnContext>,
allow_parallel_read_only: bool,
read_only_tasks: JoinSet<(usize, Result<ResponseInputItem, String>)>,
read_only_meta: HashMap<usize, (String, ToolPayload, String)>,
read_only_tasks: JoinSet<(usize, ResponseInputItem)>,
processed_items: Vec<ProcessedResponseItem>,
}
impl ToolCallExecutor {
pub(crate) fn new(
router: Arc<Router>,
session: Arc<Session>, // todo why
turn_context: Arc<TurnContext>, // todo why
session: Arc<Session>,
turn_context: Arc<TurnContext>,
) -> Self {
let allow_parallel_read_only = router.has_read_only_tools()
&& turn_context.tools_config.enable_parallel_read_only
&& turn_context.client.supports_parallel_read_only_tools();
let allow_parallel_read_only =
router.has_read_only_tools() && turn_context.tools_config.enable_parallel_read_only;
Self {
router,
@@ -57,24 +53,14 @@ impl ToolCallExecutor {
turn_context,
allow_parallel_read_only,
read_only_tasks: JoinSet::new(),
read_only_meta: HashMap::new(),
processed_items: Vec::new(),
}
}
pub(crate) fn specs(&self) -> &[ToolSpec] {
self.router.specs()
}
pub(crate) fn allow_parallel_read_only(&self) -> bool {
self.allow_parallel_read_only
}
pub(crate) fn drain_ready(&mut self) {
while let Some(res) = self.read_only_tasks.try_join_next() {
match res {
Ok((idx, Ok(response))) => self.assign_parallel_success(idx, response),
Ok((idx, Err(err))) => self.assign_parallel_failure(idx, err),
Ok((idx, response)) => self.assign_result(idx, response),
Err(join_err) => {
warn!(
?join_err,
@@ -88,8 +74,7 @@ impl ToolCallExecutor {
pub(crate) async fn flush(&mut self) {
while let Some(res) = self.read_only_tasks.join_next().await {
match res {
Ok((idx, Ok(response))) => self.assign_parallel_success(idx, response),
Ok((idx, Err(err))) => self.assign_parallel_failure(idx, err),
Ok((idx, response)) => self.assign_result(idx, response),
Err(join_err) => {
warn!(
?join_err,
@@ -172,18 +157,20 @@ impl ToolCallExecutor {
Ok(())
}
fn assign_result(&mut self, idx: usize, response: ResponseInputItem) {
if let Some(slot) = self.processed_items.get_mut(idx) {
slot.response = Some(response);
} else {
warn!(idx, "parallel tool completion missing output slot");
}
}
pub(crate) fn take_processed_items(mut self) -> Vec<ProcessedResponseItem> {
self.drain_ready();
self.processed_items
}
fn schedule_parallel_task(&mut self, idx: usize, call: ToolCall, sub_id: &str) {
let payload_clone = call.payload.clone();
self.read_only_meta.insert(
idx,
(call.call_id.clone(), payload_clone, call.tool_name.clone()),
);
let router_for_task = self.router.clone();
let session_for_task = self.session.clone();
let turn_context_for_task = self.turn_context.clone();
@@ -191,6 +178,9 @@ impl ToolCallExecutor {
self.read_only_tasks.spawn(async move {
let mut tracker = TurnDiffTracker::new();
let payload_for_fallback = call.payload.clone();
let call_id_for_fallback = call.call_id.clone();
let tool_name_for_msg = call.tool_name.clone();
let fut = async {
router_for_task
.dispatch_tool_call(
@@ -203,12 +193,16 @@ impl ToolCallExecutor {
.await
};
let result = AssertUnwindSafe(fut)
.catch_unwind()
.await
.map_err(Self::panic_to_message);
let response = match AssertUnwindSafe(fut).catch_unwind().await {
Ok(resp) => resp,
Err(panic) => {
let msg = Self::panic_to_message(panic);
let message = format!("{tool_name_for_msg} failed: {msg}");
Self::fallback_response(call_id_for_fallback, payload_for_fallback, message)
}
};
(idx, result)
(idx, response)
});
}
@@ -248,40 +242,6 @@ impl ToolCallExecutor {
Ok(())
}
fn assign_parallel_success(&mut self, idx: usize, response: ResponseInputItem) {
self.read_only_meta.remove(&idx);
if let Some(slot) = self.processed_items.get_mut(idx) {
slot.response = Some(response);
} else {
warn!(idx, "parallel tool completion missing output slot");
}
}
fn assign_parallel_failure(&mut self, idx: usize, reason: String) {
let (call_id, payload, tool_name) = self.read_only_meta.remove(&idx).unwrap_or_else(|| {
(
String::new(),
ToolPayload::Function {
arguments: String::new(),
},
String::from("unknown"),
)
});
let message = if tool_name == "unknown" {
reason
} else {
format!("{tool_name} failed: {reason}")
};
let response = Self::fallback_response(call_id, payload, message);
if let Some(slot) = self.processed_items.get_mut(idx) {
slot.response = Some(response);
} else {
warn!(idx, "parallel tool failure missing output slot");
}
}
fn fallback_response(
call_id: String,
payload: ToolPayload,

View File

@@ -1279,7 +1279,7 @@ async fn history_dedupes_streamed_and_final_messages_across_turns() {
}
#[tokio::test]
async fn parallel_tool_calls_enabled_when_supported() {
async fn parallel_tool_calls_field_not_sent() {
let server = MockServer::start().await;
let template = ResponseTemplate::new(200)
@@ -1295,7 +1295,7 @@ async fn parallel_tool_calls_enabled_when_supported() {
let mut provider = built_in_model_providers()["openai"].clone();
provider.base_url = Some(format!("{}/v1", server.uri()));
provider.supports_parallel_tool_calls = true;
provider.supports_parallel_tool_calls = true; // no longer affects payload
let provider_clone = provider.clone();
let TestCodex { codex, .. } = test_codex()
@@ -1303,7 +1303,7 @@ async fn parallel_tool_calls_enabled_when_supported() {
config.model = "gpt-5".to_string();
config.model_family = find_family_for_model("gpt-5").expect("model family");
config.enable_parallel_read_only_tools = true;
config.model_provider = provider_clone.clone();
config.model_provider = provider_clone;
config.model_provider_id = "openai".to_string();
})
.build(&server)
@@ -1322,8 +1322,6 @@ async fn parallel_tool_calls_enabled_when_supported() {
let request = &server.received_requests().await.expect("requests")[0];
let request_body = request.body_json::<serde_json::Value>().unwrap();
assert_eq!(
request_body.get("parallel_tool_calls"),
Some(&serde_json::Value::Bool(true))
);
// Field removed: executor handles scheduling internally
assert!(request_body.get("parallel_tool_calls").is_none());
}

View File

@@ -169,7 +169,6 @@ async fn compact_resume_and_fork_preserve_model_history_view() {
],
"tools": tool_calls,
"tool_choice": "auto",
"parallel_tool_calls": false,
"reasoning": {
"summary": "auto"
},
@@ -238,7 +237,6 @@ async fn compact_resume_and_fork_preserve_model_history_view() {
],
"tools": [],
"tool_choice": "auto",
"parallel_tool_calls": false,
"reasoning": {
"summary": "auto"
},
@@ -303,7 +301,6 @@ SUMMARY_ONLY_CONTEXT"
],
"tools": tool_calls,
"tool_choice": "auto",
"parallel_tool_calls": false,
"reasoning": {
"summary": "auto"
},
@@ -388,7 +385,6 @@ SUMMARY_ONLY_CONTEXT"
],
"tools": tool_calls,
"tool_choice": "auto",
"parallel_tool_calls": false,
"reasoning": {
"summary": "auto"
},
@@ -473,7 +469,6 @@ SUMMARY_ONLY_CONTEXT"
],
"tools": tool_calls,
"tool_choice": "auto",
"parallel_tool_calls": false,
"reasoning": {
"summary": "auto"
},