From 1489c9d2ef8330e8199cbbc7ed191461779ad700 Mon Sep 17 00:00:00 2001 From: shijie-openai Date: Mon, 16 Mar 2026 15:30:37 -0700 Subject: [PATCH 1/4] Persist latest model and reasoning effort in sqlite --- codex-rs/core/src/realtime_context_tests.rs | 2 ++ .../0020_threads_model_reasoning_effort.sql | 2 ++ codex-rs/state/src/extract.rs | 13 +++++++-- codex-rs/state/src/model/thread_metadata.rs | 23 +++++++++++++++ codex-rs/state/src/runtime/memories.rs | 2 ++ codex-rs/state/src/runtime/test_support.rs | 4 +++ codex-rs/state/src/runtime/threads.rs | 28 +++++++++++++++++-- 7 files changed, 70 insertions(+), 4 deletions(-) create mode 100644 codex-rs/state/migrations/0020_threads_model_reasoning_effort.sql diff --git a/codex-rs/core/src/realtime_context_tests.rs b/codex-rs/core/src/realtime_context_tests.rs index b23c2743cf2..552718a0b50 100644 --- a/codex-rs/core/src/realtime_context_tests.rs +++ b/codex-rs/core/src/realtime_context_tests.rs @@ -27,6 +27,8 @@ fn thread_metadata(cwd: &str, title: &str, first_user_message: &str) -> ThreadMe agent_nickname: None, agent_role: None, model_provider: "test-provider".to_string(), + model: Some("gpt-5".to_string()), + reasoning_effort: None, cwd: PathBuf::from(cwd), cli_version: "test".to_string(), title: title.to_string(), diff --git a/codex-rs/state/migrations/0020_threads_model_reasoning_effort.sql b/codex-rs/state/migrations/0020_threads_model_reasoning_effort.sql new file mode 100644 index 00000000000..b15f4be37f5 --- /dev/null +++ b/codex-rs/state/migrations/0020_threads_model_reasoning_effort.sql @@ -0,0 +1,2 @@ +ALTER TABLE threads ADD COLUMN model TEXT; +ALTER TABLE threads ADD COLUMN reasoning_effort TEXT; diff --git a/codex-rs/state/src/extract.rs b/codex-rs/state/src/extract.rs index ba425adbec9..6b6d57849b7 100644 --- a/codex-rs/state/src/extract.rs +++ b/codex-rs/state/src/extract.rs @@ -70,6 +70,8 @@ fn apply_turn_context(metadata: &mut ThreadMetadata, turn_ctx: &TurnContextItem) if metadata.cwd.as_os_str().is_empty() { metadata.cwd = turn_ctx.cwd.clone(); } + metadata.model = Some(turn_ctx.model.clone()); + metadata.reasoning_effort = turn_ctx.effort; metadata.sandbox_policy = enum_to_string(&turn_ctx.sandbox_policy); metadata.approval_mode = enum_to_string(&turn_ctx.approval_policy); } @@ -141,6 +143,7 @@ mod tests { use codex_protocol::config_types::ReasoningSummary; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; + use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::RolloutItem; @@ -285,6 +288,8 @@ mod tests { ); assert_eq!(metadata.cwd, PathBuf::from("/child/worktree")); + assert_eq!(metadata.model.as_deref(), Some("gpt-5")); + assert_eq!(metadata.reasoning_effort, None); assert_eq!( metadata.sandbox_policy, super::enum_to_string(&SandboxPolicy::DangerFullAccess) @@ -293,7 +298,7 @@ mod tests { } #[test] - fn turn_context_sets_cwd_when_session_cwd_missing() { + fn turn_context_sets_cwd_model_and_reasoning_effort_when_session_cwd_missing() { let mut metadata = metadata_for_test(); metadata.cwd = PathBuf::new(); @@ -312,7 +317,7 @@ mod tests { personality: None, collaboration_mode: None, realtime_active: None, - effort: None, + effort: Some(ReasoningEffort::High), summary: ReasoningSummary::Auto, user_instructions: None, developer_instructions: None, @@ -323,6 +328,8 @@ mod tests { ); assert_eq!(metadata.cwd, PathBuf::from("/fallback/workspace")); + assert_eq!(metadata.model.as_deref(), Some("gpt-5")); + assert_eq!(metadata.reasoning_effort, Some(ReasoningEffort::High)); } fn metadata_for_test() -> ThreadMetadata { @@ -337,6 +344,8 @@ mod tests { agent_nickname: None, agent_role: None, model_provider: "openai".to_string(), + model: None, + reasoning_effort: None, cwd: PathBuf::from("/tmp"), cli_version: "0.0.0".to_string(), title: String::new(), diff --git a/codex-rs/state/src/model/thread_metadata.rs b/codex-rs/state/src/model/thread_metadata.rs index c4362a8df22..c1f47349e4f 100644 --- a/codex-rs/state/src/model/thread_metadata.rs +++ b/codex-rs/state/src/model/thread_metadata.rs @@ -3,6 +3,7 @@ use chrono::DateTime; use chrono::Timelike; use chrono::Utc; use codex_protocol::ThreadId; +use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionSource; @@ -70,6 +71,10 @@ pub struct ThreadMetadata { pub agent_role: Option, /// The model provider identifier. pub model_provider: String, + /// The latest observed model for the thread. + pub model: Option, + /// The latest observed reasoning effort for the thread. + pub reasoning_effort: Option, /// The working directory for the thread. pub cwd: PathBuf, /// Version of the CLI that created the thread. @@ -181,6 +186,8 @@ impl ThreadMetadataBuilder { .model_provider .clone() .unwrap_or_else(|| default_provider.to_string()), + model: None, + reasoning_effort: None, cwd: self.cwd.clone(), cli_version: self.cli_version.clone().unwrap_or_default(), title: String::new(), @@ -237,6 +244,12 @@ impl ThreadMetadata { if self.model_provider != other.model_provider { diffs.push("model_provider"); } + if self.model != other.model { + diffs.push("model"); + } + if self.reasoning_effort != other.reasoning_effort { + diffs.push("reasoning_effort"); + } if self.cwd != other.cwd { diffs.push("cwd"); } @@ -288,6 +301,8 @@ pub(crate) struct ThreadRow { agent_nickname: Option, agent_role: Option, model_provider: String, + model: Option, + reasoning_effort: Option, cwd: String, cli_version: String, title: String, @@ -312,6 +327,8 @@ impl ThreadRow { agent_nickname: row.try_get("agent_nickname")?, agent_role: row.try_get("agent_role")?, model_provider: row.try_get("model_provider")?, + model: row.try_get("model")?, + reasoning_effort: row.try_get("reasoning_effort")?, cwd: row.try_get("cwd")?, cli_version: row.try_get("cli_version")?, title: row.try_get("title")?, @@ -340,6 +357,8 @@ impl TryFrom for ThreadMetadata { agent_nickname, agent_role, model_provider, + model, + reasoning_effort, cwd, cli_version, title, @@ -361,6 +380,10 @@ impl TryFrom for ThreadMetadata { agent_nickname, agent_role, model_provider, + model, + reasoning_effort: reasoning_effort + .map(|value| serde_json::from_value(serde_json::Value::String(value))) + .transpose()?, cwd: PathBuf::from(cwd), cli_version, title, diff --git a/codex-rs/state/src/runtime/memories.rs b/codex-rs/state/src/runtime/memories.rs index 0072e34fe79..60d5eb88391 100644 --- a/codex-rs/state/src/runtime/memories.rs +++ b/codex-rs/state/src/runtime/memories.rs @@ -167,6 +167,8 @@ SELECT agent_nickname, agent_role, model_provider, + model, + reasoning_effort, cwd, cli_version, title, diff --git a/codex-rs/state/src/runtime/test_support.rs b/codex-rs/state/src/runtime/test_support.rs index d749fe2bfba..229ece64b49 100644 --- a/codex-rs/state/src/runtime/test_support.rs +++ b/codex-rs/state/src/runtime/test_support.rs @@ -5,6 +5,8 @@ use chrono::Utc; #[cfg(test)] use codex_protocol::ThreadId; #[cfg(test)] +use codex_protocol::openai_models::ReasoningEffort; +#[cfg(test)] use codex_protocol::protocol::AskForApproval; #[cfg(test)] use codex_protocol::protocol::SandboxPolicy; @@ -49,6 +51,8 @@ pub(super) fn test_thread_metadata( agent_nickname: None, agent_role: None, model_provider: "test-provider".to_string(), + model: Some("gpt-5".to_string()), + reasoning_effort: Some(ReasoningEffort::Medium), cwd, cli_version: "0.0.0".to_string(), title: String::new(), diff --git a/codex-rs/state/src/runtime/threads.rs b/codex-rs/state/src/runtime/threads.rs index 7d63776e2a1..cdbea3f9583 100644 --- a/codex-rs/state/src/runtime/threads.rs +++ b/codex-rs/state/src/runtime/threads.rs @@ -13,6 +13,8 @@ SELECT agent_nickname, agent_role, model_provider, + model, + reasoning_effort, cwd, cli_version, title, @@ -125,6 +127,8 @@ SELECT agent_nickname, agent_role, model_provider, + model, + reasoning_effort, cwd, cli_version, title, @@ -223,6 +227,8 @@ INSERT INTO threads ( agent_nickname, agent_role, model_provider, + model, + reasoning_effort, cwd, cli_version, title, @@ -236,7 +242,7 @@ INSERT INTO threads ( git_branch, git_origin_url, memory_mode -) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) +) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(id) DO NOTHING "#, ) @@ -248,6 +254,13 @@ ON CONFLICT(id) DO NOTHING .bind(metadata.agent_nickname.as_deref()) .bind(metadata.agent_role.as_deref()) .bind(metadata.model_provider.as_str()) + .bind(metadata.model.as_deref()) + .bind( + metadata + .reasoning_effort + .as_ref() + .map(crate::extract::enum_to_string), + ) .bind(metadata.cwd.display().to_string()) .bind(metadata.cli_version.as_str()) .bind(metadata.title.as_str()) @@ -337,6 +350,8 @@ INSERT INTO threads ( agent_nickname, agent_role, model_provider, + model, + reasoning_effort, cwd, cli_version, title, @@ -350,7 +365,7 @@ INSERT INTO threads ( git_branch, git_origin_url, memory_mode -) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) +) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(id) DO UPDATE SET rollout_path = excluded.rollout_path, created_at = excluded.created_at, @@ -359,6 +374,8 @@ ON CONFLICT(id) DO UPDATE SET agent_nickname = excluded.agent_nickname, agent_role = excluded.agent_role, model_provider = excluded.model_provider, + model = excluded.model, + reasoning_effort = excluded.reasoning_effort, cwd = excluded.cwd, cli_version = excluded.cli_version, title = excluded.title, @@ -381,6 +398,13 @@ ON CONFLICT(id) DO UPDATE SET .bind(metadata.agent_nickname.as_deref()) .bind(metadata.agent_role.as_deref()) .bind(metadata.model_provider.as_str()) + .bind(metadata.model.as_deref()) + .bind( + metadata + .reasoning_effort + .as_ref() + .map(crate::extract::enum_to_string), + ) .bind(metadata.cwd.display().to_string()) .bind(metadata.cli_version.as_str()) .bind(metadata.title.as_str()) From c992e04eaac81c5d0bd83c439ab55be1c6349090 Mon Sep 17 00:00:00 2001 From: shijie-openai Date: Mon, 16 Mar 2026 20:15:34 -0700 Subject: [PATCH 2/4] better approach --- codex-rs/protocol/src/openai_models.rs | 31 ++++++++++++++++++ codex-rs/state/src/extract.rs | 35 +++++++++++++++++++-- codex-rs/state/src/model/thread_metadata.rs | 2 +- 3 files changed, 64 insertions(+), 4 deletions(-) diff --git a/codex-rs/protocol/src/openai_models.rs b/codex-rs/protocol/src/openai_models.rs index 04ca8dc9def..c84bcc93d8a 100644 --- a/codex-rs/protocol/src/openai_models.rs +++ b/codex-rs/protocol/src/openai_models.rs @@ -4,6 +4,7 @@ //! are used to preserve compatibility when older payloads omit newly introduced attributes. use std::collections::HashMap; +use std::str::FromStr; use schemars::JsonSchema; use serde::Deserialize; @@ -48,6 +49,22 @@ pub enum ReasoningEffort { XHigh, } +impl FromStr for ReasoningEffort { + type Err = String; + + fn from_str(s: &str) -> Result { + match s { + "none" => Ok(Self::None), + "minimal" => Ok(Self::Minimal), + "low" => Ok(Self::Low), + "medium" => Ok(Self::Medium), + "high" => Ok(Self::High), + "xhigh" => Ok(Self::XHigh), + _ => Err(format!("invalid reasoning_effort: {s}")), + } + } +} + /// Canonical user-input modality tags advertised by a model. #[derive( Debug, @@ -552,6 +569,20 @@ mod tests { } } + #[test] + fn reasoning_effort_from_str_accepts_known_values() { + assert_eq!("high".parse(), Ok(ReasoningEffort::High)); + assert_eq!("minimal".parse(), Ok(ReasoningEffort::Minimal)); + } + + #[test] + fn reasoning_effort_from_str_rejects_unknown_values() { + assert_eq!( + "unsupported".parse::(), + Err("invalid reasoning_effort: unsupported".to_string()) + ); + } + #[test] fn get_model_instructions_uses_template_when_placeholder_present() { let model = test_model(Some(ModelMessages { diff --git a/codex-rs/state/src/extract.rs b/codex-rs/state/src/extract.rs index 6b6d57849b7..7d68a1a2555 100644 --- a/codex-rs/state/src/extract.rs +++ b/codex-rs/state/src/extract.rs @@ -288,8 +288,6 @@ mod tests { ); assert_eq!(metadata.cwd, PathBuf::from("/child/worktree")); - assert_eq!(metadata.model.as_deref(), Some("gpt-5")); - assert_eq!(metadata.reasoning_effort, None); assert_eq!( metadata.sandbox_policy, super::enum_to_string(&SandboxPolicy::DangerFullAccess) @@ -298,7 +296,7 @@ mod tests { } #[test] - fn turn_context_sets_cwd_model_and_reasoning_effort_when_session_cwd_missing() { + fn turn_context_sets_cwd_when_session_cwd_missing() { let mut metadata = metadata_for_test(); metadata.cwd = PathBuf::new(); @@ -328,6 +326,37 @@ mod tests { ); assert_eq!(metadata.cwd, PathBuf::from("/fallback/workspace")); + } + + #[test] + fn turn_context_sets_model_and_reasoning_effort() { + let mut metadata = metadata_for_test(); + + apply_rollout_item( + &mut metadata, + &RolloutItem::TurnContext(TurnContextItem { + turn_id: Some("turn-1".to_string()), + trace_id: None, + cwd: PathBuf::from("/fallback/workspace"), + current_date: None, + timezone: None, + approval_policy: AskForApproval::OnRequest, + sandbox_policy: SandboxPolicy::new_read_only_policy(), + network: None, + model: "gpt-5".to_string(), + personality: None, + collaboration_mode: None, + realtime_active: None, + effort: Some(ReasoningEffort::High), + summary: ReasoningSummary::Auto, + user_instructions: None, + developer_instructions: None, + final_output_json_schema: None, + truncation_policy: None, + }), + "test-provider", + ); + assert_eq!(metadata.model.as_deref(), Some("gpt-5")); assert_eq!(metadata.reasoning_effort, Some(ReasoningEffort::High)); } diff --git a/codex-rs/state/src/model/thread_metadata.rs b/codex-rs/state/src/model/thread_metadata.rs index c1f47349e4f..da627466ee2 100644 --- a/codex-rs/state/src/model/thread_metadata.rs +++ b/codex-rs/state/src/model/thread_metadata.rs @@ -382,7 +382,7 @@ impl TryFrom for ThreadMetadata { model_provider, model, reasoning_effort: reasoning_effort - .map(|value| serde_json::from_value(serde_json::Value::String(value))) + .map(|value| value.parse::().map_err(anyhow::Error::msg)) .transpose()?, cwd: PathBuf::from(cwd), cli_version, From 871bf6041c35793f35c15aa0cf3d000ade3ad88b Mon Sep 17 00:00:00 2001 From: shijie-openai Date: Mon, 16 Mar 2026 22:21:55 -0700 Subject: [PATCH 3/4] More testing --- codex-rs/state/src/extract.rs | 32 +++++ codex-rs/state/src/model/thread_metadata.rs | 3 +- codex-rs/state/src/runtime/threads.rs | 135 ++++++++++++++++++++ 3 files changed, 168 insertions(+), 2 deletions(-) diff --git a/codex-rs/state/src/extract.rs b/codex-rs/state/src/extract.rs index 7d68a1a2555..037b1f5d22a 100644 --- a/codex-rs/state/src/extract.rs +++ b/codex-rs/state/src/extract.rs @@ -361,6 +361,38 @@ mod tests { assert_eq!(metadata.reasoning_effort, Some(ReasoningEffort::High)); } + #[test] + fn session_meta_does_not_set_model_or_reasoning_effort() { + let mut metadata = metadata_for_test(); + let thread_id = metadata.id; + + apply_rollout_item( + &mut metadata, + &RolloutItem::SessionMeta(SessionMetaLine { + meta: SessionMeta { + id: thread_id, + forked_from_id: None, + timestamp: "2026-02-26T00:00:00.000Z".to_string(), + cwd: PathBuf::from("/workspace"), + originator: "codex_cli_rs".to_string(), + cli_version: "0.0.0".to_string(), + source: SessionSource::Cli, + agent_nickname: None, + agent_role: None, + model_provider: Some("openai".to_string()), + base_instructions: None, + dynamic_tools: None, + memory_mode: None, + }, + git: None, + }), + "test-provider", + ); + + assert_eq!(metadata.model, None); + assert_eq!(metadata.reasoning_effort, None); + } + fn metadata_for_test() -> ThreadMetadata { let id = ThreadId::from_string(&Uuid::from_u128(42).to_string()).expect("thread id"); let created_at = DateTime::::from_timestamp(1_735_689_600, 0).expect("timestamp"); diff --git a/codex-rs/state/src/model/thread_metadata.rs b/codex-rs/state/src/model/thread_metadata.rs index da627466ee2..9aa23a0fc9e 100644 --- a/codex-rs/state/src/model/thread_metadata.rs +++ b/codex-rs/state/src/model/thread_metadata.rs @@ -382,8 +382,7 @@ impl TryFrom for ThreadMetadata { model_provider, model, reasoning_effort: reasoning_effort - .map(|value| value.parse::().map_err(anyhow::Error::msg)) - .transpose()?, + .and_then(|value| value.parse::().ok()), cwd: PathBuf::from(cwd), cli_version, title, diff --git a/codex-rs/state/src/runtime/threads.rs b/codex-rs/state/src/runtime/threads.rs index cdbea3f9583..b8de53160fe 100644 --- a/codex-rs/state/src/runtime/threads.rs +++ b/codex-rs/state/src/runtime/threads.rs @@ -780,6 +780,69 @@ mod tests { assert_eq!(memory_mode.as_deref(), Some("polluted")); } + #[tokio::test] + async fn apply_rollout_items_without_turn_context_persists_null_model_and_reasoning_effort() { + let codex_home = unique_temp_dir(); + let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string()) + .await + .expect("state db should initialize"); + let thread_id = + ThreadId::from_string("00000000-0000-0000-0000-000000000455").expect("valid thread id"); + let rollout_path = codex_home.join(format!("rollout-{thread_id}.jsonl")); + let created_at = DateTime::::from_timestamp(1_700_000_000, 0).expect("timestamp"); + let builder = ThreadMetadataBuilder::new( + thread_id, + rollout_path.clone(), + created_at, + SessionSource::Cli, + ); + let items = vec![RolloutItem::SessionMeta(SessionMetaLine { + meta: SessionMeta { + id: thread_id, + forked_from_id: None, + timestamp: created_at.to_rfc3339(), + cwd: PathBuf::from("/workspace"), + originator: "codex_cli_rs".to_string(), + cli_version: "0.0.0".to_string(), + source: SessionSource::Cli, + agent_nickname: None, + agent_role: None, + model_provider: Some("openai".to_string()), + base_instructions: None, + dynamic_tools: None, + memory_mode: None, + }, + git: None, + })]; + + runtime + .apply_rollout_items(&builder, &items, None, None) + .await + .expect("apply_rollout_items should succeed"); + + let persisted = runtime + .get_thread(thread_id) + .await + .expect("thread should load") + .expect("thread should exist"); + assert_eq!(persisted.model, None); + assert_eq!(persisted.reasoning_effort, None); + + let model: Option = sqlx::query_scalar("SELECT model FROM threads WHERE id = ?") + .bind(thread_id.to_string()) + .fetch_one(runtime.pool.as_ref()) + .await + .expect("model column should load"); + let reasoning_effort: Option = + sqlx::query_scalar("SELECT reasoning_effort FROM threads WHERE id = ?") + .bind(thread_id.to_string()) + .fetch_one(runtime.pool.as_ref()) + .await + .expect("reasoning_effort column should load"); + assert_eq!(model, None); + assert_eq!(reasoning_effort, None); + } + #[tokio::test] async fn apply_rollout_items_preserves_existing_git_branch_and_fills_missing_git_fields() { let codex_home = unique_temp_dir(); @@ -948,6 +1011,78 @@ mod tests { ); } + #[tokio::test] + async fn get_thread_ignores_unknown_reasoning_effort_values() { + let codex_home = unique_temp_dir(); + let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string()) + .await + .expect("state db should initialize"); + let thread_id = + ThreadId::from_string("00000000-0000-0000-0000-000000000793").expect("valid thread id"); + let metadata = test_thread_metadata(&codex_home, thread_id, codex_home.clone()); + + runtime + .upsert_thread(&metadata) + .await + .expect("initial upsert should succeed"); + sqlx::query("UPDATE threads SET reasoning_effort = ? WHERE id = ?") + .bind("future") + .bind(thread_id.to_string()) + .execute(runtime.pool.as_ref()) + .await + .expect("reasoning_effort update should succeed"); + + let persisted = runtime + .get_thread(thread_id) + .await + .expect("thread should load") + .expect("thread should exist"); + assert_eq!(persisted.model.as_deref(), Some("gpt-5")); + assert_eq!(persisted.reasoning_effort, None); + } + + #[tokio::test] + async fn list_threads_ignores_unknown_reasoning_effort_values() { + let codex_home = unique_temp_dir(); + let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string()) + .await + .expect("state db should initialize"); + let thread_id = + ThreadId::from_string("00000000-0000-0000-0000-000000000794").expect("valid thread id"); + let metadata = test_thread_metadata(&codex_home, thread_id, codex_home.clone()); + + runtime + .upsert_thread(&metadata) + .await + .expect("initial upsert should succeed"); + sqlx::query("UPDATE threads SET reasoning_effort = ? WHERE id = ?") + .bind("future") + .bind(thread_id.to_string()) + .execute(runtime.pool.as_ref()) + .await + .expect("reasoning_effort update should succeed"); + + let page = runtime + .list_threads( + 10, + None, + SortKey::UpdatedAt, + &["cli".to_string()], + None, + false, + None, + ) + .await + .expect("list_threads should succeed"); + let persisted = page + .items + .into_iter() + .find(|item| item.id == thread_id) + .expect("thread should be present"); + assert_eq!(persisted.model.as_deref(), Some("gpt-5")); + assert_eq!(persisted.reasoning_effort, None); + } + #[tokio::test] async fn update_thread_git_info_can_clear_fields() { let codex_home = unique_temp_dir(); From 499ee25efaa609bd1016b520e37b89a24f1a39d4 Mon Sep 17 00:00:00 2001 From: shijie-openai Date: Tue, 17 Mar 2026 09:16:25 -0700 Subject: [PATCH 4/4] Address review comments --- codex-rs/protocol/src/openai_models.rs | 11 +- codex-rs/state/src/model/thread_metadata.rs | 84 ++++++++++++ codex-rs/state/src/runtime/threads.rs | 135 -------------------- 3 files changed, 86 insertions(+), 144 deletions(-) diff --git a/codex-rs/protocol/src/openai_models.rs b/codex-rs/protocol/src/openai_models.rs index c84bcc93d8a..a113460c1a2 100644 --- a/codex-rs/protocol/src/openai_models.rs +++ b/codex-rs/protocol/src/openai_models.rs @@ -53,15 +53,8 @@ impl FromStr for ReasoningEffort { type Err = String; fn from_str(s: &str) -> Result { - match s { - "none" => Ok(Self::None), - "minimal" => Ok(Self::Minimal), - "low" => Ok(Self::Low), - "medium" => Ok(Self::Medium), - "high" => Ok(Self::High), - "xhigh" => Ok(Self::XHigh), - _ => Err(format!("invalid reasoning_effort: {s}")), - } + serde_json::from_value(serde_json::Value::String(s.to_string())) + .map_err(|_| format!("invalid reasoning_effort: {s}")) } } diff --git a/codex-rs/state/src/model/thread_metadata.rs b/codex-rs/state/src/model/thread_metadata.rs index 9aa23a0fc9e..db4a2d95e78 100644 --- a/codex-rs/state/src/model/thread_metadata.rs +++ b/codex-rs/state/src/model/thread_metadata.rs @@ -426,3 +426,87 @@ pub struct BackfillStats { /// The number of rows that failed to upsert. pub failed: usize, } + +#[cfg(test)] +mod tests { + use super::ThreadMetadata; + use super::ThreadRow; + use chrono::DateTime; + use chrono::Utc; + use codex_protocol::ThreadId; + use codex_protocol::openai_models::ReasoningEffort; + use pretty_assertions::assert_eq; + use std::path::PathBuf; + + fn thread_row(reasoning_effort: Option<&str>) -> ThreadRow { + ThreadRow { + id: "00000000-0000-0000-0000-000000000123".to_string(), + rollout_path: "/tmp/rollout-123.jsonl".to_string(), + created_at: 1_700_000_000, + updated_at: 1_700_000_100, + source: "cli".to_string(), + agent_nickname: None, + agent_role: None, + model_provider: "openai".to_string(), + model: Some("gpt-5".to_string()), + reasoning_effort: reasoning_effort.map(str::to_string), + cwd: "/tmp/workspace".to_string(), + cli_version: "0.0.0".to_string(), + title: String::new(), + sandbox_policy: "read-only".to_string(), + approval_mode: "on-request".to_string(), + tokens_used: 1, + first_user_message: String::new(), + archived_at: None, + git_sha: None, + git_branch: None, + git_origin_url: None, + } + } + + fn expected_thread_metadata(reasoning_effort: Option) -> ThreadMetadata { + ThreadMetadata { + id: ThreadId::from_string("00000000-0000-0000-0000-000000000123") + .expect("valid thread id"), + rollout_path: PathBuf::from("/tmp/rollout-123.jsonl"), + created_at: DateTime::::from_timestamp(1_700_000_000, 0).expect("timestamp"), + updated_at: DateTime::::from_timestamp(1_700_000_100, 0).expect("timestamp"), + source: "cli".to_string(), + agent_nickname: None, + agent_role: None, + model_provider: "openai".to_string(), + model: Some("gpt-5".to_string()), + reasoning_effort, + cwd: PathBuf::from("/tmp/workspace"), + cli_version: "0.0.0".to_string(), + title: String::new(), + sandbox_policy: "read-only".to_string(), + approval_mode: "on-request".to_string(), + tokens_used: 1, + first_user_message: None, + archived_at: None, + git_sha: None, + git_branch: None, + git_origin_url: None, + } + } + + #[test] + fn thread_row_parses_reasoning_effort() { + let metadata = ThreadMetadata::try_from(thread_row(Some("high"))) + .expect("thread metadata should parse"); + + assert_eq!( + metadata, + expected_thread_metadata(Some(ReasoningEffort::High)) + ); + } + + #[test] + fn thread_row_ignores_unknown_reasoning_effort_values() { + let metadata = ThreadMetadata::try_from(thread_row(Some("future"))) + .expect("thread metadata should parse"); + + assert_eq!(metadata, expected_thread_metadata(None)); + } +} diff --git a/codex-rs/state/src/runtime/threads.rs b/codex-rs/state/src/runtime/threads.rs index b8de53160fe..cdbea3f9583 100644 --- a/codex-rs/state/src/runtime/threads.rs +++ b/codex-rs/state/src/runtime/threads.rs @@ -780,69 +780,6 @@ mod tests { assert_eq!(memory_mode.as_deref(), Some("polluted")); } - #[tokio::test] - async fn apply_rollout_items_without_turn_context_persists_null_model_and_reasoning_effort() { - let codex_home = unique_temp_dir(); - let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string()) - .await - .expect("state db should initialize"); - let thread_id = - ThreadId::from_string("00000000-0000-0000-0000-000000000455").expect("valid thread id"); - let rollout_path = codex_home.join(format!("rollout-{thread_id}.jsonl")); - let created_at = DateTime::::from_timestamp(1_700_000_000, 0).expect("timestamp"); - let builder = ThreadMetadataBuilder::new( - thread_id, - rollout_path.clone(), - created_at, - SessionSource::Cli, - ); - let items = vec![RolloutItem::SessionMeta(SessionMetaLine { - meta: SessionMeta { - id: thread_id, - forked_from_id: None, - timestamp: created_at.to_rfc3339(), - cwd: PathBuf::from("/workspace"), - originator: "codex_cli_rs".to_string(), - cli_version: "0.0.0".to_string(), - source: SessionSource::Cli, - agent_nickname: None, - agent_role: None, - model_provider: Some("openai".to_string()), - base_instructions: None, - dynamic_tools: None, - memory_mode: None, - }, - git: None, - })]; - - runtime - .apply_rollout_items(&builder, &items, None, None) - .await - .expect("apply_rollout_items should succeed"); - - let persisted = runtime - .get_thread(thread_id) - .await - .expect("thread should load") - .expect("thread should exist"); - assert_eq!(persisted.model, None); - assert_eq!(persisted.reasoning_effort, None); - - let model: Option = sqlx::query_scalar("SELECT model FROM threads WHERE id = ?") - .bind(thread_id.to_string()) - .fetch_one(runtime.pool.as_ref()) - .await - .expect("model column should load"); - let reasoning_effort: Option = - sqlx::query_scalar("SELECT reasoning_effort FROM threads WHERE id = ?") - .bind(thread_id.to_string()) - .fetch_one(runtime.pool.as_ref()) - .await - .expect("reasoning_effort column should load"); - assert_eq!(model, None); - assert_eq!(reasoning_effort, None); - } - #[tokio::test] async fn apply_rollout_items_preserves_existing_git_branch_and_fills_missing_git_fields() { let codex_home = unique_temp_dir(); @@ -1011,78 +948,6 @@ mod tests { ); } - #[tokio::test] - async fn get_thread_ignores_unknown_reasoning_effort_values() { - let codex_home = unique_temp_dir(); - let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string()) - .await - .expect("state db should initialize"); - let thread_id = - ThreadId::from_string("00000000-0000-0000-0000-000000000793").expect("valid thread id"); - let metadata = test_thread_metadata(&codex_home, thread_id, codex_home.clone()); - - runtime - .upsert_thread(&metadata) - .await - .expect("initial upsert should succeed"); - sqlx::query("UPDATE threads SET reasoning_effort = ? WHERE id = ?") - .bind("future") - .bind(thread_id.to_string()) - .execute(runtime.pool.as_ref()) - .await - .expect("reasoning_effort update should succeed"); - - let persisted = runtime - .get_thread(thread_id) - .await - .expect("thread should load") - .expect("thread should exist"); - assert_eq!(persisted.model.as_deref(), Some("gpt-5")); - assert_eq!(persisted.reasoning_effort, None); - } - - #[tokio::test] - async fn list_threads_ignores_unknown_reasoning_effort_values() { - let codex_home = unique_temp_dir(); - let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string()) - .await - .expect("state db should initialize"); - let thread_id = - ThreadId::from_string("00000000-0000-0000-0000-000000000794").expect("valid thread id"); - let metadata = test_thread_metadata(&codex_home, thread_id, codex_home.clone()); - - runtime - .upsert_thread(&metadata) - .await - .expect("initial upsert should succeed"); - sqlx::query("UPDATE threads SET reasoning_effort = ? WHERE id = ?") - .bind("future") - .bind(thread_id.to_string()) - .execute(runtime.pool.as_ref()) - .await - .expect("reasoning_effort update should succeed"); - - let page = runtime - .list_threads( - 10, - None, - SortKey::UpdatedAt, - &["cli".to_string()], - None, - false, - None, - ) - .await - .expect("list_threads should succeed"); - let persisted = page - .items - .into_iter() - .find(|item| item.id == thread_id) - .expect("thread should be present"); - assert_eq!(persisted.model.as_deref(), Some("gpt-5")); - assert_eq!(persisted.reasoning_effort, None); - } - #[tokio::test] async fn update_thread_git_info_can_clear_fields() { let codex_home = unique_temp_dir();