mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Preserve server reasoning summary and verbosity settings in the TUI (#49144)
## Why Forwarding local defaults as request overrides can replace server defaults or saved thread settings for reasoning summaries and verbosity. ## What changed Only forward `model_reasoning_summary` and `model_verbosity` as shared thread request overrides when their winning configuration layer is a session flag or an explicitly selected user profile. Use the effective values from that layer, and omit values whose winning source is ordinary user or project configuration. Preserve the existing reasoning-summary defaults for new embedded threads. ## Testing Add coverage for profile, project, and CLI precedence. Extend outbound model request tests to verify that remote-mode thread start and resume preserve server reasoning summary and verbosity settings despite conflicting local values. GitOrigin-RevId: 5e14254a51ed7506118a46847bb7466ab58908ef
This commit is contained in:
@@ -1836,18 +1836,6 @@ fn config_request_overrides_from_config(
|
||||
.as_ref()
|
||||
.map(std::string::ToString::to_string),
|
||||
);
|
||||
insert(
|
||||
"model_reasoning_summary",
|
||||
config
|
||||
.model_reasoning_summary
|
||||
.map(|summary| summary.to_string()),
|
||||
);
|
||||
insert(
|
||||
"model_verbosity",
|
||||
config
|
||||
.model_verbosity
|
||||
.map(|verbosity| verbosity.to_string()),
|
||||
);
|
||||
insert(
|
||||
"personality",
|
||||
personality_opt_out_only(config.personality).map(|personality| personality.to_string()),
|
||||
@@ -1856,6 +1844,23 @@ fn config_request_overrides_from_config(
|
||||
"web_search",
|
||||
Some(config.web_search_mode.value().to_string()),
|
||||
);
|
||||
// Only winning launch choices may replace server defaults or saved thread settings.
|
||||
let origins = config.config_layer_stack.origins();
|
||||
let effective = config.config_layer_stack.effective_config();
|
||||
for key in ["model_reasoning_summary", "model_verbosity"] {
|
||||
if origins.get(key).is_some_and(|origin| {
|
||||
matches!(
|
||||
origin.name,
|
||||
ConfigLayerSource::SessionFlags
|
||||
| ConfigLayerSource::User {
|
||||
profile: Some(_),
|
||||
..
|
||||
}
|
||||
)
|
||||
}) {
|
||||
overrides.insert(key.to_string(), serde_json::json!(effective[key]));
|
||||
}
|
||||
}
|
||||
if config.bypass_hook_trust {
|
||||
overrides.insert("bypass_hook_trust".to_string(), true.into());
|
||||
}
|
||||
@@ -3381,12 +3386,11 @@ mod tests {
|
||||
let string = |value: &str| serde_json::Value::String(value.to_string());
|
||||
let expected_config = HashMap::from([
|
||||
("model_reasoning_effort".to_string(), string("high")),
|
||||
("model_reasoning_summary".to_string(), string("detailed")),
|
||||
("model_verbosity".to_string(), string("low")),
|
||||
("web_search".to_string(), string("disabled")),
|
||||
("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}),
|
||||
@@ -3396,6 +3400,71 @@ mod tests {
|
||||
assert_eq!(fork.config, Some(expected_config));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn config_overrides_forward_explicit_summary_and_verbosity() -> Result<()> {
|
||||
let home = tempfile::tempdir()?;
|
||||
let workspace = home.path().join("workspace");
|
||||
std::fs::create_dir_all(workspace.join(".codex"))?;
|
||||
std::fs::write(
|
||||
home.path().join("config.toml"),
|
||||
format!(
|
||||
"model_reasoning_summary = \"concise\"\nmodel_verbosity = \"low\"\n[projects.{}]\ntrust_level = \"trusted\"\n",
|
||||
toml::Value::String(workspace.to_string_lossy().into_owned()),
|
||||
),
|
||||
)?;
|
||||
let profile = AbsolutePathBuf::from_absolute_path(home.path().join("work.config.toml"))?;
|
||||
std::fs::write(
|
||||
&profile,
|
||||
"model_reasoning_summary = \"detailed\"\nmodel_verbosity = \"high\"\n",
|
||||
)?;
|
||||
for (selected_profile, project, cli, expected) in [
|
||||
(false, false, false, [None, None]),
|
||||
(true, false, false, [Some("detailed"), Some("high")]),
|
||||
(false, false, true, [Some("auto"), Some("medium")]),
|
||||
(true, false, true, [Some("auto"), Some("medium")]),
|
||||
(true, true, false, [None, None]),
|
||||
(true, true, true, [Some("auto"), Some("medium")]),
|
||||
] {
|
||||
std::fs::write(
|
||||
workspace.join(".codex/config.toml"),
|
||||
if project {
|
||||
"model_reasoning_summary = \"concise\"\nmodel_verbosity = \"low\"\n"
|
||||
} else {
|
||||
""
|
||||
},
|
||||
)?;
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(home.path().to_path_buf())
|
||||
.harness_overrides(ConfigOverrides {
|
||||
cwd: Some(workspace.clone()),
|
||||
..ConfigOverrides::default()
|
||||
})
|
||||
.loader_overrides(codex_config::LoaderOverrides {
|
||||
user_config_path: selected_profile.then(|| profile.clone()),
|
||||
user_config_profile: selected_profile.then(|| "work".parse().unwrap()),
|
||||
..codex_config::LoaderOverrides::without_managed_config_for_tests()
|
||||
})
|
||||
.cli_overrides(if cli {
|
||||
vec![
|
||||
("model_reasoning_summary".to_string(), "auto".into()),
|
||||
("model_verbosity".to_string(), "medium".into()),
|
||||
]
|
||||
} else {
|
||||
Vec::new()
|
||||
})
|
||||
.build()
|
||||
.await?;
|
||||
let overrides =
|
||||
config_request_overrides_from_config(&config).expect("config overrides");
|
||||
assert_eq!(
|
||||
["model_reasoning_summary", "model_verbosity"]
|
||||
.map(|key| overrides.get(key).and_then(serde_json::Value::as_str)),
|
||||
expected,
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_resume_params_can_restore_persisted_model_settings() {
|
||||
let temp_dir = tempfile::tempdir().expect("tempdir");
|
||||
@@ -3418,16 +3487,10 @@ mod tests {
|
||||
assert_eq!(params.model_provider, None);
|
||||
assert_eq!(
|
||||
params.config,
|
||||
Some(HashMap::from([
|
||||
(
|
||||
"model_reasoning_summary".to_string(),
|
||||
serde_json::Value::String("detailed".to_string()),
|
||||
),
|
||||
(
|
||||
"web_search".to_string(),
|
||||
serde_json::Value::String("cached".to_string()),
|
||||
),
|
||||
]))
|
||||
Some(HashMap::from([(
|
||||
"web_search".to_string(),
|
||||
serde_json::Value::String("cached".to_string()),
|
||||
)]))
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -1,58 +1,63 @@
|
||||
//! Verify embedded TUI reasoning configuration and metadata on its outbound model request.
|
||||
//! Verify TUI reasoning defaults and server-owned settings on outbound model requests.
|
||||
|
||||
use super::*;
|
||||
use crate::legacy_core::config::ConfigBuilder;
|
||||
use codex_app_server_protocol::ServerNotification;
|
||||
use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::config_types::Verbosity;
|
||||
use core_test_support::responses;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
|
||||
#[tokio::test]
|
||||
async fn embedded_reasoning_defaults_reach_responses() -> Result<()> {
|
||||
for (settings, summary, stream_options) in [
|
||||
("", json!(null), json!(null)),
|
||||
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!(null),
|
||||
json!(null),
|
||||
ThreadParamsMode::Embedded,
|
||||
),
|
||||
(
|
||||
"[features]\nconcurrent_reasoning_summaries = true",
|
||||
json!(null),
|
||||
json!(null),
|
||||
ThreadParamsMode::Embedded,
|
||||
),
|
||||
(
|
||||
"model_reasoning_summary = 'detailed'",
|
||||
json!("detailed"),
|
||||
json!(null),
|
||||
ThreadParamsMode::Embedded,
|
||||
),
|
||||
(
|
||||
"model_reasoning_summary = 'detailed'\n[features]\nconcurrent_reasoning_summaries = true",
|
||||
json!("detailed"),
|
||||
json!({"reasoning_summary_delivery": "sequential_cutoff"}),
|
||||
ThreadParamsMode::Embedded,
|
||||
),
|
||||
(
|
||||
"model_reasoning_summary = 'none'\n[features]\nconcurrent_reasoning_summaries = true",
|
||||
json!(null),
|
||||
json!(null),
|
||||
ThreadParamsMode::Embedded,
|
||||
),
|
||||
(
|
||||
"model_reasoning_summary = 'detailed'\nmodel_verbosity = 'high'",
|
||||
json!("detailed"),
|
||||
json!(null),
|
||||
ThreadParamsMode::Remote,
|
||||
),
|
||||
] {
|
||||
let server = responses::start_mock_server().await;
|
||||
let response = responses::mount_sse_once(
|
||||
&server,
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("response"),
|
||||
responses::ev_completed("response"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let home = tempfile::tempdir()?;
|
||||
let base_url = server.uri();
|
||||
std::fs::write(
|
||||
home.path().join("config.toml"),
|
||||
format!(
|
||||
r#"
|
||||
model = "gpt-5.2"
|
||||
model = "gpt-5.5"
|
||||
model_provider = "reasoning-test"
|
||||
{settings}
|
||||
[model_providers.reasoning-test]
|
||||
@@ -64,72 +69,106 @@ stream_max_retries = 0
|
||||
"#
|
||||
),
|
||||
)?;
|
||||
let config = ConfigBuilder::default()
|
||||
let mut config = ConfigBuilder::default()
|
||||
.codex_home(home.path().to_path_buf())
|
||||
.build()
|
||||
.await?;
|
||||
let mut app_server = crate::start_embedded_app_server_for_picker(&config).await?;
|
||||
app_server.thread_params_mode = mode;
|
||||
if mode == ThreadParamsMode::Remote {
|
||||
config.model_reasoning_summary = Some(ReasoningSummary::Concise);
|
||||
config.model_verbosity = Some(Verbosity::Low);
|
||||
}
|
||||
let started = app_server.start_thread(&config).await?;
|
||||
let turn = app_server
|
||||
.turn_start(
|
||||
started.session.thread_id,
|
||||
"tui-user-message".to_string(),
|
||||
vec![UserInput::Text {
|
||||
text: "hello".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
config.cwd.to_path_buf(),
|
||||
/*approval_policy*/ None,
|
||||
/*approvals_reviewer*/ None,
|
||||
TurnPermissionsOverride::Preserve,
|
||||
&config.workspace_roots,
|
||||
started.session.model.clone(),
|
||||
/*effort*/ None,
|
||||
/*summary*/ None,
|
||||
/*service_tier*/ None,
|
||||
/*collaboration_mode*/ None,
|
||||
/*personality*/ None,
|
||||
/*output_schema*/ None,
|
||||
)
|
||||
.await?;
|
||||
tokio::time::timeout(std::time::Duration::from_secs(/*secs*/ 30), async {
|
||||
while let Some(event) = app_server.next_event().await {
|
||||
if let AppServerEvent::ServerNotification(notification) = event
|
||||
&& let ServerNotification::TurnCompleted(completed) = *notification
|
||||
{
|
||||
assert_eq!(
|
||||
completed.turn.status,
|
||||
codex_app_server_protocol::TurnStatus::Completed
|
||||
);
|
||||
return;
|
||||
for resume in [false, true] {
|
||||
if resume {
|
||||
if mode != ThreadParamsMode::Remote {
|
||||
break;
|
||||
}
|
||||
app_server.shutdown().await?;
|
||||
app_server = crate::start_embedded_app_server_for_picker(&config).await?;
|
||||
app_server.thread_params_mode = mode;
|
||||
app_server
|
||||
.resume_thread(
|
||||
&crate::local_settings::LocalSettings::from(&config),
|
||||
config.clone(),
|
||||
started.session.thread_id,
|
||||
ResumeModelSettings::RestoreFromThread,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
panic!("app-server disconnected before completing the turn");
|
||||
})
|
||||
.await?;
|
||||
let body = response.single_request().body_json();
|
||||
assert_eq!(
|
||||
json!({"summary": body["reasoning"]["summary"], "stream_options": body["stream_options"]}),
|
||||
json!({"summary": summary, "stream_options": stream_options}),
|
||||
"settings: {settings}"
|
||||
);
|
||||
let metadata: serde_json::Value = serde_json::from_str(
|
||||
body["client_metadata"]["x-codex-turn-metadata"]
|
||||
.as_str()
|
||||
.expect("canonical turn metadata"),
|
||||
)?;
|
||||
assert_eq!(
|
||||
(
|
||||
&metadata["thread_id"],
|
||||
&metadata["turn_id"],
|
||||
&metadata["turn_trigger"],
|
||||
),
|
||||
(
|
||||
&json!(started.session.thread_id),
|
||||
&json!(turn.turn.id),
|
||||
&json!("user"),
|
||||
let response = responses::mount_sse_once(
|
||||
&server,
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("response"),
|
||||
responses::ev_completed("response"),
|
||||
]),
|
||||
)
|
||||
);
|
||||
.await;
|
||||
let turn = app_server
|
||||
.turn_start(
|
||||
started.session.thread_id,
|
||||
"tui-user-message".to_string(),
|
||||
vec![UserInput::Text {
|
||||
text: "hello".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
config.cwd.to_path_buf(),
|
||||
/*approval_policy*/ None,
|
||||
/*approvals_reviewer*/ None,
|
||||
TurnPermissionsOverride::Preserve,
|
||||
&config.workspace_roots,
|
||||
started.session.model.clone(),
|
||||
/*effort*/ None,
|
||||
/*summary*/ None,
|
||||
/*service_tier*/ None,
|
||||
/*collaboration_mode*/ None,
|
||||
/*personality*/ None,
|
||||
/*output_schema*/ None,
|
||||
)
|
||||
.await?;
|
||||
tokio::time::timeout(std::time::Duration::from_secs(/*secs*/ 30), async {
|
||||
while let Some(event) = app_server.next_event().await {
|
||||
if let AppServerEvent::ServerNotification(notification) = event
|
||||
&& let ServerNotification::TurnCompleted(completed) = *notification
|
||||
{
|
||||
assert_eq!(
|
||||
completed.turn.status,
|
||||
codex_app_server_protocol::TurnStatus::Completed
|
||||
);
|
||||
return;
|
||||
}
|
||||
}
|
||||
panic!("app-server disconnected before completing the turn");
|
||||
})
|
||||
.await?;
|
||||
let body = response.single_request().body_json();
|
||||
assert_eq!(
|
||||
json!({"summary": body["reasoning"]["summary"], "stream_options": body["stream_options"]}),
|
||||
json!({"summary": summary, "stream_options": stream_options}),
|
||||
"settings: {settings}"
|
||||
);
|
||||
if mode == ThreadParamsMode::Remote {
|
||||
assert_eq!(body["text"]["verbosity"], json!("high"));
|
||||
}
|
||||
let metadata: serde_json::Value = serde_json::from_str(
|
||||
body["client_metadata"]["x-codex-turn-metadata"]
|
||||
.as_str()
|
||||
.expect("canonical turn metadata"),
|
||||
)?;
|
||||
assert_eq!(
|
||||
(
|
||||
&metadata["thread_id"],
|
||||
&metadata["turn_id"],
|
||||
&metadata["turn_trigger"],
|
||||
),
|
||||
(
|
||||
&json!(started.session.thread_id),
|
||||
&json!(turn.turn.id),
|
||||
&json!("user"),
|
||||
)
|
||||
);
|
||||
}
|
||||
app_server.shutdown().await?;
|
||||
}
|
||||
Ok(())
|
||||
|
||||
Reference in New Issue
Block a user