mirror of
https://github.com/openai/codex.git
synced 2026-11-12 21:20:34 +00:00
Refresh derived thread titles over time
This commit is contained in:
@@ -744,8 +744,25 @@ fn stored_thread_from_state(
|
||||
thread_id,
|
||||
items: history_items.clone(),
|
||||
});
|
||||
let name = state.names.get(&thread_id).cloned().flatten();
|
||||
let metadata = state.metadata_updates.get(&thread_id);
|
||||
let preview = metadata
|
||||
.and_then(|metadata| metadata.preview.clone())
|
||||
.unwrap_or_default();
|
||||
let name = state
|
||||
.names
|
||||
.get(&thread_id)
|
||||
.cloned()
|
||||
.flatten()
|
||||
.or_else(|| {
|
||||
let title = metadata
|
||||
.and_then(|metadata| {
|
||||
metadata
|
||||
.title
|
||||
.clone()
|
||||
.or_else(|| metadata.derived_title.clone())
|
||||
})?;
|
||||
(title.trim() != preview.trim()).then_some(title)
|
||||
});
|
||||
let rollout_path = state
|
||||
.rollout_paths
|
||||
.iter()
|
||||
@@ -761,9 +778,7 @@ fn stored_thread_from_state(
|
||||
.or(rollout_path),
|
||||
forked_from_id: created.forked_from_id,
|
||||
parent_thread_id: created.parent_thread_id,
|
||||
preview: metadata
|
||||
.and_then(|metadata| metadata.preview.clone())
|
||||
.unwrap_or_default(),
|
||||
preview,
|
||||
name,
|
||||
model_provider: metadata
|
||||
.and_then(|metadata| metadata.model_provider.clone())
|
||||
|
||||
@@ -470,6 +470,56 @@ mod tests {
|
||||
assert_eq!(metadata.title, "observed append");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn live_thread_refreshes_title_from_later_user_messages() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let config = test_config(home.path());
|
||||
let runtime = codex_state::StateRuntime::init(
|
||||
config.sqlite_home.clone(),
|
||||
config.default_model_provider_id.clone(),
|
||||
)
|
||||
.await
|
||||
.expect("state db should initialize");
|
||||
let store = Arc::new(LocalThreadStore::new(config, Some(runtime.clone())));
|
||||
let thread_id = ThreadId::default();
|
||||
let live_thread = LiveThread::create(store.clone(), create_thread_params(thread_id))
|
||||
.await
|
||||
.expect("create live thread");
|
||||
|
||||
live_thread
|
||||
.append_items(&[user_message_item("initial setup task")])
|
||||
.await
|
||||
.expect("append first user message");
|
||||
live_thread
|
||||
.append_items(&[user_message_item("switch to release notes")])
|
||||
.await
|
||||
.expect("append later user message");
|
||||
live_thread.flush().await.expect("flush thread");
|
||||
|
||||
let metadata = runtime
|
||||
.get_thread(thread_id)
|
||||
.await
|
||||
.expect("sqlite metadata read")
|
||||
.expect("sqlite metadata");
|
||||
assert_eq!(
|
||||
metadata.first_user_message.as_deref(),
|
||||
Some("initial setup task")
|
||||
);
|
||||
assert_eq!(metadata.preview.as_deref(), Some("initial setup task"));
|
||||
assert_eq!(metadata.title, "switch to release notes");
|
||||
|
||||
let thread = store
|
||||
.read_thread(ReadThreadParams {
|
||||
thread_id,
|
||||
include_archived: false,
|
||||
include_history: false,
|
||||
})
|
||||
.await
|
||||
.expect("read thread");
|
||||
assert_eq!(thread.preview, "initial setup task");
|
||||
assert_eq!(thread.name.as_deref(), Some("switch to release notes"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn live_thread_output_advances_updated_at_but_not_recency_at() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
@@ -728,6 +778,7 @@ mod tests {
|
||||
metadata.first_user_message.as_deref(),
|
||||
Some("Hello from user")
|
||||
);
|
||||
assert_eq!(metadata.title, "new live append");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -12,6 +12,7 @@ use codex_rollout::ARCHIVED_SESSIONS_SUBDIR;
|
||||
use codex_rollout::append_rollout_item_to_path;
|
||||
use codex_rollout::append_thread_name;
|
||||
use codex_rollout::find_archived_thread_path_by_id_str;
|
||||
use codex_rollout::find_thread_name_by_id;
|
||||
use codex_rollout::find_thread_path_by_id_str;
|
||||
use codex_rollout::read_session_meta_line;
|
||||
use codex_state::ThreadMetadataBuilder;
|
||||
@@ -253,6 +254,11 @@ async fn apply_metadata_update(
|
||||
if let Some(preview) = patch.preview {
|
||||
metadata.preview = Some(preview);
|
||||
}
|
||||
if let Some(derived_title) = patch.derived_title
|
||||
&& !has_explicit_thread_name(store, thread_id).await
|
||||
{
|
||||
metadata.title = derived_title;
|
||||
}
|
||||
if let Some(name) = patch.name {
|
||||
metadata.title = name.unwrap_or_default();
|
||||
}
|
||||
@@ -482,6 +488,7 @@ fn has_observed_metadata_facts(patch: &ThreadMetadataPatch) -> bool {
|
||||
patch.rollout_path.is_some()
|
||||
|| patch.preview.is_some()
|
||||
|| patch.title.is_some()
|
||||
|| patch.derived_title.is_some()
|
||||
|| patch.model_provider.is_some()
|
||||
|| patch.model.is_some()
|
||||
|| patch.reasoning_effort.is_some()
|
||||
@@ -499,6 +506,14 @@ fn has_observed_metadata_facts(patch: &ThreadMetadataPatch) -> bool {
|
||||
|| patch.first_user_message.is_some()
|
||||
}
|
||||
|
||||
async fn has_explicit_thread_name(store: &LocalThreadStore, thread_id: ThreadId) -> bool {
|
||||
find_thread_name_by_id(store.config.codex_home.as_path(), &thread_id)
|
||||
.await
|
||||
.ok()
|
||||
.flatten()
|
||||
.is_some_and(|name| !name.trim().is_empty())
|
||||
}
|
||||
|
||||
fn enum_to_string<T: serde::Serialize>(value: &T) -> String {
|
||||
match serde_json::to_value(value) {
|
||||
Ok(serde_json::Value::String(value)) => value,
|
||||
@@ -1444,6 +1459,56 @@ mod tests {
|
||||
assert_eq!(thread.name.as_deref(), Some("Derived first message"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn metadata_patch_does_not_apply_derived_title_over_existing_name() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let config = test_config(home.path());
|
||||
let runtime = codex_state::StateRuntime::init(
|
||||
home.path().to_path_buf(),
|
||||
config.default_model_provider_id.clone(),
|
||||
)
|
||||
.await
|
||||
.expect("state db should initialize");
|
||||
let store = LocalThreadStore::new(config, Some(runtime.clone()));
|
||||
let uuid = Uuid::from_u128(307);
|
||||
let thread_id = ThreadId::from_string(&uuid.to_string()).expect("valid thread id");
|
||||
write_session_file(home.path(), "2025-01-03T16-15-00", uuid).expect("session file");
|
||||
|
||||
store
|
||||
.update_thread_metadata(UpdateThreadMetadataParams {
|
||||
thread_id,
|
||||
patch: ThreadMetadataPatch {
|
||||
name: Some(Some("User chosen name".to_string())),
|
||||
..Default::default()
|
||||
},
|
||||
include_archived: false,
|
||||
})
|
||||
.await
|
||||
.expect("set explicit name");
|
||||
|
||||
let thread = store
|
||||
.update_thread_metadata(UpdateThreadMetadataParams {
|
||||
thread_id,
|
||||
patch: ThreadMetadataPatch {
|
||||
derived_title: Some("Later live prompt".to_string()),
|
||||
preview: Some("Original prompt".to_string()),
|
||||
first_user_message: Some("Original prompt".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
include_archived: false,
|
||||
})
|
||||
.await
|
||||
.expect("apply observed metadata");
|
||||
|
||||
assert_eq!(thread.name.as_deref(), Some("User chosen name"));
|
||||
let metadata = runtime
|
||||
.get_thread(thread_id)
|
||||
.await
|
||||
.expect("sqlite metadata read")
|
||||
.expect("sqlite metadata");
|
||||
assert_eq!(metadata.title, "User chosen name");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn metadata_patch_applies_latest_preview_and_first_user_message() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
|
||||
@@ -37,7 +37,6 @@ pub(crate) struct ThreadMetadataSync {
|
||||
cwd_seen: bool,
|
||||
preview_seen: bool,
|
||||
first_user_message_seen: bool,
|
||||
title_seen: bool,
|
||||
pending_update: Option<ThreadMetadataPatch>,
|
||||
pending_update_generation: u64,
|
||||
last_touch_persisted_at: Option<Instant>,
|
||||
@@ -83,7 +82,6 @@ impl ThreadMetadataSync {
|
||||
cwd_seen: !cwd.as_os_str().is_empty(),
|
||||
preview_seen: false,
|
||||
first_user_message_seen: false,
|
||||
title_seen: false,
|
||||
pending_update: Some(update),
|
||||
pending_update_generation: 1,
|
||||
last_touch_persisted_at: None,
|
||||
@@ -102,7 +100,6 @@ impl ThreadMetadataSync {
|
||||
.is_some_and(|cwd| !cwd.as_os_str().is_empty()),
|
||||
preview_seen: false,
|
||||
first_user_message_seen: false,
|
||||
title_seen: false,
|
||||
pending_update: None,
|
||||
pending_update_generation: 0,
|
||||
last_touch_persisted_at: None,
|
||||
@@ -259,12 +256,9 @@ impl ThreadMetadataSync {
|
||||
update.preview = Some(preview);
|
||||
}
|
||||
}
|
||||
if !self.title_seen {
|
||||
let title = strip_user_message_prefix(user.message.as_str());
|
||||
if !title.is_empty() {
|
||||
self.title_seen = true;
|
||||
update.title = Some(title.to_string());
|
||||
}
|
||||
let title = strip_user_message_prefix(user.message.as_str());
|
||||
if !title.is_empty() {
|
||||
update.derived_title = Some(title.to_string());
|
||||
}
|
||||
}
|
||||
RolloutItem::EventMsg(EventMsg::TokenCount(token_count)) => {
|
||||
@@ -357,6 +351,7 @@ fn update_has_metadata_facts(update: &ThreadMetadataPatch) -> bool {
|
||||
update.rollout_path.is_some()
|
||||
|| update.preview.is_some()
|
||||
|| update.title.is_some()
|
||||
|| update.derived_title.is_some()
|
||||
|| update.model_provider.is_some()
|
||||
|| update.model.is_some()
|
||||
|| update.reasoning_effort.is_some()
|
||||
@@ -422,7 +417,10 @@ mod tests {
|
||||
"2025-01-03T12:00:00+00:00"
|
||||
);
|
||||
assert_eq!(update.patch.preview.as_deref(), Some("hello metadata"));
|
||||
assert_eq!(update.patch.title.as_deref(), Some("hello metadata"));
|
||||
assert_eq!(
|
||||
update.patch.derived_title.as_deref(),
|
||||
Some("hello metadata")
|
||||
);
|
||||
assert_eq!(
|
||||
update.patch.first_user_message.as_deref(),
|
||||
Some("hello metadata")
|
||||
@@ -457,11 +455,14 @@ mod tests {
|
||||
update.patch.first_user_message.as_deref(),
|
||||
Some("first user text")
|
||||
);
|
||||
assert_eq!(update.patch.title.as_deref(), Some("first user text"));
|
||||
assert_eq!(
|
||||
update.patch.derived_title.as_deref(),
|
||||
Some("first user text")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn later_user_messages_do_not_emit_existing_preview_fields() {
|
||||
fn later_user_messages_update_title_without_existing_preview_fields() {
|
||||
let thread_id = ThreadId::new();
|
||||
let mut sync = ThreadMetadataSync::for_resume(&resume_params(
|
||||
thread_id,
|
||||
@@ -479,11 +480,37 @@ mod tests {
|
||||
.expect("updated_at touch");
|
||||
|
||||
assert_eq!(update.patch.preview, None);
|
||||
assert_eq!(update.patch.title, None);
|
||||
assert_eq!(
|
||||
update.patch.derived_title.as_deref(),
|
||||
Some("later user text")
|
||||
);
|
||||
assert_eq!(update.patch.first_user_message, None);
|
||||
assert!(update.patch.updated_at.is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resume_history_derives_title_from_latest_user_message() {
|
||||
let thread_id = ThreadId::new();
|
||||
let sync = ThreadMetadataSync::for_resume(&resume_params(
|
||||
thread_id,
|
||||
vec![
|
||||
RolloutItem::EventMsg(EventMsg::UserMessage(user_message("first user text"))),
|
||||
RolloutItem::EventMsg(EventMsg::UserMessage(user_message("latest user text"))),
|
||||
],
|
||||
));
|
||||
|
||||
let update = sync.take_pending_update().expect("pending metadata update");
|
||||
assert_eq!(update.patch.preview.as_deref(), Some("first user text"));
|
||||
assert_eq!(
|
||||
update.patch.first_user_message.as_deref(),
|
||||
Some("first user text")
|
||||
);
|
||||
assert_eq!(
|
||||
update.patch.derived_title.as_deref(),
|
||||
Some("latest user text")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn metadata_irrelevant_items_coalesce_updated_at_touches() {
|
||||
let thread_id = ThreadId::new();
|
||||
|
||||
@@ -538,6 +538,11 @@ pub struct ThreadMetadataPatch {
|
||||
pub preview: Option<String>,
|
||||
/// Best-effort title derived from history.
|
||||
pub title: Option<String>,
|
||||
/// Best-effort live title derived from appended history.
|
||||
///
|
||||
/// Unlike `title`, stores may ignore this when a user-facing name has
|
||||
/// already been set explicitly.
|
||||
pub derived_title: Option<String>,
|
||||
/// Model provider associated with the thread.
|
||||
pub model_provider: Option<String>,
|
||||
/// Latest observed model.
|
||||
@@ -617,6 +622,9 @@ impl ThreadMetadataPatch {
|
||||
if next.title.is_some() {
|
||||
self.title = next.title;
|
||||
}
|
||||
if next.derived_title.is_some() {
|
||||
self.derived_title = next.derived_title;
|
||||
}
|
||||
if next.model_provider.is_some() {
|
||||
self.model_provider = next.model_provider;
|
||||
}
|
||||
@@ -683,6 +691,7 @@ impl ThreadMetadataPatch {
|
||||
&& self.rollout_path.is_none()
|
||||
&& self.preview.is_none()
|
||||
&& self.title.is_none()
|
||||
&& self.derived_title.is_none()
|
||||
&& self.model_provider.is_none()
|
||||
&& self.model.is_none()
|
||||
&& self.reasoning_effort.is_none()
|
||||
@@ -832,6 +841,7 @@ mod tests {
|
||||
name: Some(None),
|
||||
preview: None,
|
||||
title: Some("new title".to_string()),
|
||||
derived_title: Some("derived title".to_string()),
|
||||
git_info: Some(GitInfoPatch {
|
||||
sha: None,
|
||||
branch: Some(Some("feature".to_string())),
|
||||
@@ -843,6 +853,7 @@ mod tests {
|
||||
assert_eq!(current.name, Some(None));
|
||||
assert_eq!(current.preview.as_deref(), Some("old preview"));
|
||||
assert_eq!(current.title.as_deref(), Some("new title"));
|
||||
assert_eq!(current.derived_title.as_deref(), Some("derived title"));
|
||||
assert_eq!(
|
||||
current.git_info,
|
||||
Some(GitInfoPatch {
|
||||
|
||||
Reference in New Issue
Block a user