mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
chore: simplify user message detection (#10611)
We don't check anymore the response item with `user` role as they may be instructions etc
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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 = "<turn_aborted>";
|
||||
|
||||
/// 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<String> {
|
||||
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<T: Serialize>(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!("<prior context> {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::<Utc>::from_timestamp(1_735_689_600, 0).expect("timestamp");
|
||||
|
||||
Reference in New Issue
Block a user