From 38f6c6b1141699e87fffb398372abe157e6c52c1 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Wed, 4 Feb 2026 11:14:53 +0000 Subject: [PATCH] chore: simplify user message detection (#10611) We don't check anymore the response item with `user` role as they may be instructions etc --- codex-rs/core/src/rollout/list.rs | 2 +- codex-rs/core/src/rollout/recorder.rs | 2 + codex-rs/state/src/extract.rs | 119 +------------------------- 3 files changed, 6 insertions(+), 117 deletions(-) diff --git a/codex-rs/core/src/rollout/list.rs b/codex-rs/core/src/rollout/list.rs index 5e7c1aa35..34e144e56 100644 --- a/codex-rs/core/src/rollout/list.rs +++ b/codex-rs/core/src/rollout/list.rs @@ -1125,7 +1125,7 @@ async fn find_thread_path_by_id_str_in_subdir( let found = results.matches.into_iter().next().map(|m| m.full_path()); if found.is_some() { tracing::error!("state db missing rollout path for thread {id_str}"); - state_db::record_discrepancy("find_thread_path_by_id_str_in_subdir", "path_mismatch"); + state_db::record_discrepancy("find_thread_path_by_id_str_in_subdir", "falling_back"); } Ok(found) diff --git a/codex-rs/core/src/rollout/recorder.rs b/codex-rs/core/src/rollout/recorder.rs index ab017810f..465bcbbcd 100644 --- a/codex-rs/core/src/rollout/recorder.rs +++ b/codex-rs/core/src/rollout/recorder.rs @@ -187,6 +187,8 @@ impl RolloutRecorder { populate_thread_heads(page.items.as_mut_slice()).await; return Ok(page); } + tracing::error!("Falling back on rollout system"); + state_db::record_discrepancy("list_threads_with_db_fallback", "falling_back"); if archived { let root = codex_home.join(ARCHIVED_SESSIONS_SUBDIR); diff --git a/codex-rs/state/src/extract.rs b/codex-rs/state/src/extract.rs index bfcd75416..2bf394975 100644 --- a/codex-rs/state/src/extract.rs +++ b/codex-rs/state/src/extract.rs @@ -1,21 +1,13 @@ use crate::model::ThreadMetadata; -use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; -use codex_protocol::models::is_local_image_close_tag_text; -use codex_protocol::models::is_local_image_open_tag_text; -use codex_protocol::protocol::ENVIRONMENT_CONTEXT_OPEN_TAG; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::SessionMetaLine; use codex_protocol::protocol::TurnContextItem; -use codex_protocol::protocol::USER_INSTRUCTIONS_OPEN_TAG; use codex_protocol::protocol::USER_MESSAGE_BEGIN; use serde::Serialize; use serde_json::Value; -const USER_INSTRUCTIONS_PREFIX: &str = "# AGENTS.md instructions for "; -const TURN_ABORTED_OPEN_TAG: &str = ""; - /// Apply a rollout item to the metadata structure. pub fn apply_rollout_item( metadata: &mut ThreadMetadata, @@ -78,73 +70,8 @@ fn apply_event_msg(metadata: &mut ThreadMetadata, event: &EventMsg) { } } -fn apply_response_item(metadata: &mut ThreadMetadata, item: &ResponseItem) { - if !is_user_response_item(item) { - return; - } - metadata.has_user_event = true; - if metadata.title.is_empty() - && let Some(text) = extract_user_message_text(item) - { - metadata.title = text; - } -} - -// TODO(jif) unify once the discussion is settled -fn is_user_response_item(item: &ResponseItem) -> bool { - let ResponseItem::Message { role, content, .. } = item else { - return false; - }; - role == "user" - && !is_user_instructions(content.as_slice()) - && !is_session_prefix_content(content.as_slice()) -} - -fn is_user_instructions(content: &[ContentItem]) -> bool { - if let [ContentItem::InputText { text }] = content { - text.starts_with(USER_INSTRUCTIONS_PREFIX) || text.starts_with(USER_INSTRUCTIONS_OPEN_TAG) - } else { - false - } -} - -fn is_session_prefix_content(content: &[ContentItem]) -> bool { - if let [ContentItem::InputText { text }] = content { - is_session_prefix(text) - } else { - false - } -} - -fn is_session_prefix(text: &str) -> bool { - let lowered = text.trim_start().to_ascii_lowercase(); - lowered.starts_with(ENVIRONMENT_CONTEXT_OPEN_TAG) || lowered.starts_with(TURN_ABORTED_OPEN_TAG) -} - -fn extract_user_message_text(item: &ResponseItem) -> Option { - let ResponseItem::Message { role, content, .. } = item else { - return None; - }; - if role != "user" { - return None; - } - let texts: Vec<&str> = content - .iter() - .filter_map(|content_item| match content_item { - ContentItem::InputText { text } => Some(text.as_str()), - ContentItem::InputImage { .. } | ContentItem::OutputText { .. } => None, - }) - .filter(|text| !is_local_image_open_tag_text(text) && !is_local_image_close_tag_text(text)) - .collect(); - if texts.is_empty() { - return None; - } - let joined = texts.join("\n"); - Some( - strip_user_message_prefix(joined.as_str()) - .trim() - .to_string(), - ) +fn apply_response_item(_metadata: &mut ThreadMetadata, _item: &ResponseItem) { + // TODO (jif) } fn strip_user_message_prefix(text: &str) -> &str { @@ -165,7 +92,6 @@ pub(crate) fn enum_to_string(value: &T) -> String { #[cfg(test)] mod tests { use super::apply_rollout_item; - use super::extract_user_message_text; use crate::model::ThreadMetadata; use chrono::DateTime; use chrono::Utc; @@ -174,31 +100,11 @@ mod tests { use codex_protocol::models::ResponseItem; use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::USER_INSTRUCTIONS_OPEN_TAG; - use codex_protocol::protocol::USER_MESSAGE_BEGIN; + use pretty_assertions::assert_eq; use std::path::PathBuf; use uuid::Uuid; - #[test] - fn extracts_user_message_text() { - let item = ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ - ContentItem::InputText { - text: format!(" {USER_MESSAGE_BEGIN}actual question"), - }, - ContentItem::InputImage { - image_url: "https://example.com/image.png".to_string(), - }, - ], - end_turn: None, - phase: None, - }; - let actual = extract_user_message_text(&item); - assert_eq!(actual.as_deref(), Some("actual question")); - } - #[test] fn user_instructions_do_not_count_as_user_events() { let mut metadata = metadata_for_test(); @@ -240,25 +146,6 @@ mod tests { assert_eq!(metadata.title, ""); } - #[test] - fn image_only_user_messages_still_count_as_user_events() { - let mut metadata = metadata_for_test(); - let item = RolloutItem::ResponseItem(ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputImage { - image_url: "https://example.com/image.png".to_string(), - }], - end_turn: None, - phase: None, - }); - - apply_rollout_item(&mut metadata, &item, "test-provider"); - - assert_eq!(metadata.has_user_event, true); - assert_eq!(metadata.title, ""); - } - 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");