Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 57 additions & 4 deletions codex-rs/tui/src/app/tests/new_session_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ async fn replacement_uses_server_defaults_and_preserves_explicit_launch_settings
(Some("medium"), "effort", "server-model", "low"),
(None, "profile_model", "profile-model", "high"),
(None, "profile_effort", "server-model", "low"),
(None, "profile_summary", "server-model", "high"),
(None, "profile_summary_off", "server-model", "high"),
(None, "profile_summary_cli", "server-model", "high"),
(Some("medium"), "profile_model", "profile-model", "high"),
(Some("medium"), "profile_effort", "server-model", "low"),
(Some(""), "profile", "profile-model", "low"),
Expand All @@ -158,7 +161,7 @@ async fn replacement_uses_server_defaults_and_preserves_explicit_launch_settings
)?;
std::fs::write(
client_home.path().join("config.toml"),
"model = \"stale-client-model\"\nmodel_reasoning_effort = \"low\"\n",
"model = \"stale-client-model\"\nmodel_reasoning_effort = \"low\"\nmodel_reasoning_summary = \"detailed\"\nfeatures.concurrent_reasoning_summaries = true\n",
)?;
if let Some(effort) = managed_effort {
let effort = if effort.is_empty() {
Expand All @@ -179,7 +182,11 @@ async fn replacement_uses_server_defaults_and_preserves_explicit_launch_settings
HistoryCapabilities::Current,
/*blocked_thread_list*/ None,
/*failed_thread_name*/ None,
crate::app_server_session::ThreadParamsMode::Remote,
if explicit.starts_with("profile_summary") {
crate::app_server_session::ThreadParamsMode::Embedded
} else {
crate::app_server_session::ThreadParamsMode::Remote
},
LoaderOverrides {
user_config_path: Some(server_home.path().join("work.config.toml").abs()),
user_config_profile: Some("work".parse()?),
Expand All @@ -203,14 +210,26 @@ async fn replacement_uses_server_defaults_and_preserves_explicit_launch_settings
"model_reasoning_effort".to_string(),
TomlValue::String("low".to_string()),
)),
profile @ ("profile" | "profile_model" | "profile_effort" | "profile_unrelated") => {
profile @ ("profile"
| "profile_model"
| "profile_effort"
| "profile_unrelated"
| "profile_summary"
| "profile_summary_off"
| "profile_summary_cli") => {
let path = client_home.path().join("work.config.toml");
std::fs::write(
&path,
match profile {
"profile_model" => "model = \"profile-model\"\n",
"profile_effort" => "model_reasoning_effort = \"low\"\n",
"profile_unrelated" => "model_verbosity = \"low\"\n",
"profile_summary" => {
"model_reasoning_summary = \"detailed\"\nfeatures.concurrent_reasoning_summaries = true\n"
}
"profile_summary_off" | "profile_summary_cli" => {
"model_reasoning_summary = \"detailed\"\nfeatures.concurrent_reasoning_summaries = false\n"
}
_ => "model = \"profile-model\"\nmodel_reasoning_effort = \"low\"\n",
},
)?;
Expand All @@ -219,6 +238,21 @@ async fn replacement_uses_server_defaults_and_preserves_explicit_launch_settings
}
_ => {}
}
if explicit == "profile_summary_cli" {
app.cli_kv_overrides.push((
"features.concurrent_reasoning_summaries".into(),
true.into(),
));
}
if explicit.starts_with("profile_summary") {
app.app_server_target = AppServerTarget::LocalDaemon {
allow_embedded_fallback: true,
endpoint: crate::RemoteAppServerEndpoint::UnixSocket {
socket_path: client_home.path().join("daemon.sock").abs(),
},
};
app.open_agents_overview(&server);
}
let mut tui = crate::tui::test_support::make_test_tui()?;
app.start_fresh_session(
&mut tui,
Expand All @@ -230,6 +264,21 @@ async fn replacement_uses_server_defaults_and_preserves_explicit_launch_settings
.await;
let starts = recorded_params(&requests, "thread/start");
assert_eq!(starts.len(), 1);
assert_eq!(
(
starts[0]["config"].get("model_reasoning_summary"),
starts[0]["config"]["features"].get("concurrent_reasoning_summaries"),
),
(
explicit
.starts_with("profile_summary")
.then_some(&serde_json::json!("detailed")),
explicit
.starts_with("profile_summary")
.then_some(&serde_json::json!(explicit != "profile_summary_off")),
),
"{explicit}",
);
assert_eq!(
(
&starts[0]["model"],
Expand All @@ -243,7 +292,11 @@ async fn replacement_uses_server_defaults_and_preserves_explicit_launch_settings
assert_eq!(
recorded_params(&requests, "config/read"),
vec![
serde_json::json!({"cwd": server_config.cwd.display().to_string(), "includeLayers": true})
serde_json::json!({"cwd": if explicit.starts_with("profile_summary") {
client_home.path().display().to_string()
} else {
server_config.cwd.display().to_string()
}, "includeLayers": true})
],
);
if explicit == "saved" {
Expand Down
23 changes: 18 additions & 5 deletions codex-rs/tui/src/app/tests/startup_defaults_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,11 +427,11 @@ async fn fresh_startup_reads_destination_and_cleared_model_uses_catalog() -> Res
let launch_cwd = tempdir()?;
std::fs::write(
client_home.path().join("config.toml"),
"model = \"stale-client-model\"\nmodel_reasoning_effort = \"low\"\n",
"model = \"stale-client-model\"\nmodel_reasoning_effort = \"low\"\nmodel_reasoning_summary = \"detailed\"\nfeatures.concurrent_reasoning_summaries = true\n",
)?;
std::fs::write(
server_home.path().join("config.toml"),
"model_reasoning_effort = \"high\"\nsandbox_mode = \"read-only\"\n",
"model_reasoning_effort = \"high\"\nsandbox_mode = \"read-only\"\nmodel_reasoning_summary = \"concise\"\n",
)?;
let mut config = ConfigBuilder::default()
.codex_home(client_home.path().to_path_buf())
Expand Down Expand Up @@ -465,6 +465,10 @@ async fn fresh_startup_reads_destination_and_cleared_model_uses_catalog() -> Res
if override_cwd {
server = server.with_remote_cwd_override(Some(launch_cwd.path().to_path_buf()));
}
if !remote {
// Direct starts must inherit summaries even without a preceding defaults read.
server.start_thread(&config).await?;
}
assert!(config.config_layer_stack.is_projectless());
let bootstrap = server.bootstrap(&config).await?;
assert_eq!(bootstrap.default_model, "stale-client-model");
Expand Down Expand Up @@ -503,9 +507,18 @@ async fn fresh_startup_reads_destination_and_cleared_model_uses_catalog() -> Res
)
);
let starts = recorded_params(&requests, "thread/start");
assert_eq!(starts.len(), 1);
assert_eq!(starts[0]["model"], serde_json::Value::Null);
assert!(starts[0]["config"].get("model_reasoning_effort").is_none());
assert_eq!(starts.len(), if remote { 1 } else { 2 });
let latest = starts.last().unwrap();
assert_eq!(latest["model"], serde_json::Value::Null);
assert!(latest["config"].get("model_reasoning_effort").is_none());
for start in &starts {
assert!(start["config"].get("model_reasoning_summary").is_none());
assert!(
start["config"]["features"]
.get("concurrent_reasoning_summaries")
.is_none()
);
}
let (mut app, _, _) = make_test_app_with_channels().await;
app.chat_widget.handle_thread_session_quiet(started.session);
if !remote {
Expand Down
61 changes: 14 additions & 47 deletions codex-rs/tui/src/app_server_session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1946,8 +1946,8 @@ fn config_request_overrides_from_config(
&origins,
&mut overrides,
);
for key in ["model_reasoning_summary", "model_verbosity"] {
if origins.get(key).is_some_and(|origin| {
let is_launch = |key: &str| {
origins.get(key).is_some_and(|origin| {
matches!(
origin.name,
ConfigLayerSource::SessionFlags
Expand All @@ -1956,10 +1956,19 @@ fn config_request_overrides_from_config(
..
}
)
}) {
})
};
for key in ["model_reasoning_summary", "model_verbosity"] {
if is_launch(key) {
overrides.insert(key.to_string(), serde_json::json!(effective[key]));
}
}
if is_launch("features.concurrent_reasoning_summaries") {
overrides
.entry("features".to_string())
.or_insert_with(|| serde_json::json!({}))["concurrent_reasoning_summaries"] =
serde_json::json!(effective["features"]["concurrent_reasoning_summaries"]);
}
if config.bypass_hook_trust {
overrides.insert("bypass_hook_trust".to_string(), true.into());
}
Expand All @@ -1982,37 +1991,6 @@ fn remove_permission_config_overrides(config: &mut Option<HashMap<String, serde_
}
}

fn new_thread_reasoning_overrides(config: &Config) -> Option<HashMap<String, serde_json::Value>> {
let mut overrides = config_request_overrides_from_config(config, ThreadParamsMode::Embedded)
.unwrap_or_default();
let summary = config
.model_reasoning_summary
.unwrap_or(codex_protocol::config_types::ReasoningSummary::None);
overrides.insert(
"model_reasoning_summary".to_string(),
serde_json::Value::String(summary.to_string()),
);
let explicit_feature = config
.config_layer_stack
.effective_config()
.get("features")
.and_then(|features| features.get("concurrent_reasoning_summaries"))
.and_then(toml::Value::as_bool);
let features = overrides
.entry("features".to_string())
.or_insert_with(|| serde_json::json!({}));
if let Some(features) = features.as_object_mut() {
features.insert(
"concurrent_reasoning_summaries".to_string(),
serde_json::Value::Bool(
summary != codex_protocol::config_types::ReasoningSummary::None
&& explicit_feature.unwrap_or(/*default*/ false),
),
);
}
Some(overrides)
}

fn service_tier_override_from_config(config: &Config) -> Option<Option<String>> {
let local_settings = LocalSettings::from(config);
config.service_tier.clone().map(Some).or_else(|| {
Expand Down Expand Up @@ -2145,12 +2123,7 @@ pub(crate) fn thread_start_params_from_config(
approvals_reviewer: approvals_reviewer_override_from_config(config),
sandbox,
permissions,
config: match thread_params_mode {
ThreadParamsMode::Embedded => new_thread_reasoning_overrides(config),
ThreadParamsMode::Remote => {
config_request_overrides_from_config(config, thread_params_mode)
}
},
config: config_request_overrides_from_config(config, thread_params_mode),
ephemeral: Some(config.ephemeral),
history_mode: (!config.ephemeral).then_some(ThreadHistoryMode::Paginated),
session_start_source,
Expand Down Expand Up @@ -3491,13 +3464,7 @@ mod tests {
("model_reasoning_effort".to_string(), string("high")),
("bypass_hook_trust".to_string(), true.into()),
]);
let mut expected_start_config = expected_config.clone();
expected_start_config.insert("model_reasoning_summary".to_string(), string("detailed"));
expected_start_config.insert(
"features".to_string(),
serde_json::json!({"concurrent_reasoning_summaries": false}),
);
assert_eq!(start.config, Some(expected_start_config));
assert_eq!(start.config, Some(expected_config.clone()));
assert_eq!(resume.config, Some(expected_config.clone()));
assert_eq!(fork.config, Some(expected_config));
}
Expand Down
102 changes: 18 additions & 84 deletions codex-rs/tui/src/app_server_session/reasoning_defaults_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,22 @@ use serde_json::json;
#[tokio::test]
async fn reasoning_defaults_reach_responses() -> Result<()> {
for (settings, summary, stream_options, mode) in [
("", json!(null), json!(null), ThreadParamsMode::Embedded),
(
"[features]\nconcurrent_reasoning_summaries = false",
"",
json!("concise"),
json!(null),
ThreadParamsMode::Embedded,
),
(
"[features]\nconcurrent_reasoning_summaries = false",
json!("concise"),
json!(null),
ThreadParamsMode::Embedded,
),
(
"[features]\nconcurrent_reasoning_summaries = true",
json!(null),
json!(null),
json!("concise"),
json!({"reasoning_summary_delivery": "sequential_cutoff"}),
ThreadParamsMode::Embedded,
),
(
Expand Down Expand Up @@ -73,10 +78,18 @@ stream_max_retries = 0
codex_protocol::config_types::TrustLevel::Trusted,
)
.map_err(|error| color_eyre::eyre::eyre!(error.to_string()))?;
let server_config = ConfigBuilder::default()
let mut server_config = ConfigBuilder::default()
.codex_home(home.path().to_path_buf())
.build()
.await?;
let mut catalog = codex_models_manager::bundled_models_response()?;
catalog
.models
.iter_mut()
.find(|model| model.slug == "gpt-5.5")
.expect("bundled model")
.default_reasoning_summary = codex_protocol::config_types::ReasoningSummary::Concise;
server_config.model_catalog = Some(catalog);
let client_home = tempfile::tempdir()?;
let config = if mode == ThreadParamsMode::Remote {
std::fs::write(
Expand Down Expand Up @@ -193,82 +206,3 @@ stream_max_retries = 0
}
Ok(())
}

#[tokio::test]
async fn new_tui_threads_disable_summaries_unless_explicitly_enabled() {
let temp_dir = tempfile::tempdir().expect("tempdir");
for (config_text, expected_summary, expected_concurrent) in [
("", "none", false),
(
"[features]\nconcurrent_reasoning_summaries = true",
"none",
false,
),
("model_reasoning_summary = 'auto'", "auto", false),
("model_reasoning_summary = 'detailed'", "detailed", false),
(
"model_reasoning_summary = 'detailed'\n[features]\nconcurrent_reasoning_summaries = true",
"detailed",
true,
),
("model_reasoning_summary = 'none'", "none", false),
(
"model_reasoning_summary = 'none'\n[features]\nconcurrent_reasoning_summaries = true",
"none",
false,
),
(
"model_reasoning_summary = 'concise'\n[features]\nconcurrent_reasoning_summaries = false",
"concise",
false,
),
] {
std::fs::write(temp_dir.path().join("config.toml"), config_text).expect("config");
let config = ConfigBuilder::default()
.codex_home(temp_dir.path().to_path_buf())
.build()
.await
.expect("config should build");
let start = thread_start_params_from_config(
&config,
ThreadParamsMode::Embedded,
/*remote_cwd_override*/ None,
/*session_start_source*/ None,
);
let overrides = start.config.expect("thread config");
assert_eq!(
overrides.get("model_reasoning_summary"),
Some(&serde_json::json!(expected_summary)),
);
assert_eq!(
overrides["features"]["concurrent_reasoning_summaries"],
expected_concurrent,
);
}
std::fs::write(
temp_dir.path().join("config.toml"),
"model_reasoning_summary = 'detailed'",
)
.expect("config");
let config = ConfigBuilder::default()
.codex_home(temp_dir.path().to_path_buf())
.cli_overrides(vec![(
"model_reasoning_summary".to_string(),
toml::Value::String("none".to_string()),
)])
.build()
.await
.expect("override config");
let start = thread_start_params_from_config(
&config,
ThreadParamsMode::Embedded,
/*remote_cwd_override*/ None,
/*session_start_source*/ None,
);
let overrides = start.config.expect("thread config");
assert_eq!(overrides["model_reasoning_summary"], "none");
assert_eq!(
overrides["features"]["concurrent_reasoning_summaries"],
false
);
}
Loading
Loading