mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
Centralize context fragment kind constants (#54316)
## What changed Replace duplicated content-kind strings with constants shared by fragment producers and consumers, including authorization checks, forked-history filtering, and memory instructions. Share the compaction summary kind between core and Guardian's transcript filter. ## Testing Add a regression test verifying that Guardian excludes compaction summaries while retaining surrounding user messages, including a user message with identical text. GitOrigin-RevId: 36dbdf0f82c61b52d2571ac1c392fb151c98151b
This commit is contained in:
@@ -17,3 +17,6 @@ pub use fragment::ContextualUserFragment;
|
||||
pub use fragment::RenderedFragment;
|
||||
|
||||
pub use recap_prompt::RecapPrompt;
|
||||
|
||||
/// Shared classification for compaction summaries produced by core and filtered by Guardian.
|
||||
pub const COMPACTION_SUMMARY_KIND: &str = "compaction.summary";
|
||||
|
||||
@@ -17,6 +17,7 @@ use crate::context::ContextualUserFragment;
|
||||
use crate::context::CurrentTimeReminder;
|
||||
use crate::context::CurrentTimeUnavailable;
|
||||
use crate::context::DeveloperInstructions;
|
||||
use crate::context::GuardianApprovedAction;
|
||||
use crate::context::ManagedDeveloperInstructions;
|
||||
use crate::context::ModelInstructions;
|
||||
use crate::context::MultiAgentModeInstructions;
|
||||
@@ -141,7 +142,9 @@ fn retain_forked_developer_message(item: &mut ResponseItem, usage_hint_texts: &[
|
||||
// Persisted role hints can predate the current bundled wording and lack markers.
|
||||
if matches!(
|
||||
content_item.kind().as_str(),
|
||||
"guardian.approved_action" | "multi_agent.role_instructions" | "multi_agent.usage_hint"
|
||||
GuardianApprovedAction::KIND
|
||||
| MultiAgentRoleInstructions::KIND
|
||||
| MultiAgentUsageHint::KIND
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -7,6 +7,8 @@ pub(crate) struct CompactionSummary {
|
||||
}
|
||||
|
||||
impl CompactionSummary {
|
||||
pub(crate) const KIND: &str = codex_context_fragments::COMPACTION_SUMMARY_KIND;
|
||||
|
||||
pub(crate) fn new(summary: impl Into<String>) -> Self {
|
||||
Self {
|
||||
summary: summary.into(),
|
||||
@@ -16,7 +18,7 @@ impl CompactionSummary {
|
||||
|
||||
impl ContextualUserFragment for CompactionSummary {
|
||||
fn content_kind(&self) -> ContentItemKind {
|
||||
ContentItemKind("compaction.summary".to_string())
|
||||
ContentItemKind(Self::KIND.to_string())
|
||||
}
|
||||
|
||||
fn role(&self) -> &'static str {
|
||||
|
||||
@@ -14,10 +14,12 @@ use super::LegacyUnifiedExecProcessLimitWarning;
|
||||
use super::RecommendedPluginsInstructions;
|
||||
use super::SubagentNotification;
|
||||
use super::TurnAborted;
|
||||
use super::UnsupportedMedia;
|
||||
use super::UserGoalUpdate;
|
||||
use super::UserInstructions;
|
||||
use super::UserShellCommand;
|
||||
use super::world_state::EnvironmentsState;
|
||||
use crate::image_preparation::ImagePreparationError;
|
||||
|
||||
const CONTEXTUAL_USER_FRAGMENT_MATCHERS: &[fn(&str) -> bool] = &[
|
||||
UserInstructions::matches_text,
|
||||
@@ -73,9 +75,9 @@ pub(crate) fn is_user_authorization_message(item: &ResponseItem) -> bool {
|
||||
kind.0.as_str(),
|
||||
"" | "unknown"
|
||||
// Media preparation can replace real user input.
|
||||
| "images.preparation_error"
|
||||
| "images.unsupported"
|
||||
| "audio.unsupported"
|
||||
| ImagePreparationError::KIND
|
||||
| UnsupportedMedia::IMAGE_KIND
|
||||
| UnsupportedMedia::AUDIO_KIND
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -8,6 +8,8 @@ pub(crate) struct GuardianApprovedAction {
|
||||
}
|
||||
|
||||
impl GuardianApprovedAction {
|
||||
pub(crate) const KIND: &str = "guardian.approved_action";
|
||||
|
||||
pub(crate) fn new(approved_action_json: String) -> Self {
|
||||
Self {
|
||||
approved_action_json,
|
||||
@@ -17,7 +19,7 @@ impl GuardianApprovedAction {
|
||||
|
||||
impl ContextualUserFragment for GuardianApprovedAction {
|
||||
fn content_kind(&self) -> ContentItemKind {
|
||||
ContentItemKind("guardian.approved_action".to_string())
|
||||
ContentItemKind(Self::KIND.to_string())
|
||||
}
|
||||
|
||||
fn role(&self) -> &'static str {
|
||||
|
||||
@@ -11,6 +11,11 @@ pub enum MemoryContextFragment {
|
||||
ExtractionEvidence(String),
|
||||
}
|
||||
|
||||
impl MemoryContextFragment {
|
||||
/// Content classification for memory read-path instructions.
|
||||
pub const READ_INSTRUCTIONS_KIND: &str = "memories.instructions";
|
||||
}
|
||||
|
||||
impl ContextualUserFragment for MemoryContextFragment {
|
||||
fn role(&self) -> &'static str {
|
||||
match self {
|
||||
@@ -21,7 +26,7 @@ impl ContextualUserFragment for MemoryContextFragment {
|
||||
fn content_kind(&self) -> ContentItemKind {
|
||||
ContentItemKind(
|
||||
match self {
|
||||
Self::ReadInstructions(_) => "memories.instructions",
|
||||
Self::ReadInstructions(_) => Self::READ_INSTRUCTIONS_KIND,
|
||||
Self::ExtractionEvidence(_) => "memories.extraction_evidence",
|
||||
}
|
||||
.to_string(),
|
||||
|
||||
@@ -8,6 +8,8 @@ pub(crate) struct MultiAgentUsageHint {
|
||||
}
|
||||
|
||||
impl MultiAgentUsageHint {
|
||||
pub(crate) const KIND: &str = "multi_agent.usage_hint";
|
||||
|
||||
pub(crate) fn new(text: &str) -> Self {
|
||||
Self {
|
||||
text: text.to_string(),
|
||||
@@ -17,7 +19,7 @@ impl MultiAgentUsageHint {
|
||||
|
||||
impl ContextualUserFragment for MultiAgentUsageHint {
|
||||
fn content_kind(&self) -> ContentItemKind {
|
||||
ContentItemKind("multi_agent.usage_hint".to_string())
|
||||
ContentItemKind(Self::KIND.to_string())
|
||||
}
|
||||
|
||||
fn role(&self) -> &'static str {
|
||||
|
||||
@@ -8,14 +8,17 @@ pub(crate) struct UnsupportedMedia {
|
||||
}
|
||||
|
||||
impl UnsupportedMedia {
|
||||
pub(crate) const IMAGE_KIND: &str = "images.unsupported";
|
||||
pub(crate) const AUDIO_KIND: &str = "audio.unsupported";
|
||||
|
||||
pub(crate) const IMAGE: Self = Self {
|
||||
text: "image content omitted because you do not support image input",
|
||||
content_kind: "images.unsupported",
|
||||
content_kind: Self::IMAGE_KIND,
|
||||
};
|
||||
|
||||
pub(crate) const AUDIO: Self = Self {
|
||||
text: "audio content omitted because you do not support audio input",
|
||||
content_kind: "audio.unsupported",
|
||||
content_kind: Self::AUDIO_KIND,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -23,6 +23,7 @@ pub enum UserGoalUpdate {
|
||||
}
|
||||
|
||||
impl UserGoalUpdate {
|
||||
pub(crate) const KIND: &str = "user.goal";
|
||||
pub(crate) const OMITTED_OBJECTIVE_KIND: &str = "user.goal.omitted";
|
||||
|
||||
/// Captures accepted goal instructions at a compaction input boundary.
|
||||
@@ -53,7 +54,7 @@ impl UserGoalUpdate {
|
||||
let [ContentItem::InputText { text }] = content.as_slice() else {
|
||||
return None;
|
||||
};
|
||||
(role == "user" && matches!(kind.0.as_str(), "user.goal" | Self::OMITTED_OBJECTIVE_KIND))
|
||||
(role == "user" && matches!(kind.0.as_str(), Self::KIND | Self::OMITTED_OBJECTIVE_KIND))
|
||||
.then_some(text)
|
||||
}
|
||||
}
|
||||
@@ -71,7 +72,7 @@ impl ContextualUserFragment for UserGoalUpdate {
|
||||
} if serde_json::json!(objective).to_string().len() > MAX_OBJECTIVE_BYTES => {
|
||||
Self::OMITTED_OBJECTIVE_KIND
|
||||
}
|
||||
Self::Set { .. } | Self::Clear => "user.goal",
|
||||
Self::Set { .. } | Self::Clear => Self::KIND,
|
||||
};
|
||||
ContentItemKind(kind.to_owned())
|
||||
}
|
||||
|
||||
@@ -107,6 +107,8 @@ pub(crate) enum ImagePreparationError {
|
||||
}
|
||||
|
||||
impl ImagePreparationError {
|
||||
pub(crate) const KIND: &str = "images.preparation_error";
|
||||
|
||||
fn placeholder(&self) -> &'static str {
|
||||
match self {
|
||||
ImagePreparationError::RemoteUrlUnsupported => REMOTE_IMAGE_URL_PLACEHOLDER,
|
||||
@@ -285,7 +287,7 @@ async fn prepare_message_content(
|
||||
warn!(%error, "failed to prepare message image");
|
||||
*item = AnnotatedContent::text(
|
||||
error.placeholder(),
|
||||
ContentItemKind("images.preparation_error".to_string()),
|
||||
ContentItemKind(ImagePreparationError::KIND.to_string()),
|
||||
codex_protocol::models::ContentItemMetadata::harness(),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -94,7 +94,7 @@ impl ContextContributor for MemoriesExtension {
|
||||
.map(|instructions| {
|
||||
PromptFragment::developer_policy(
|
||||
instructions,
|
||||
ContentItemKind("memories.instructions".to_string()),
|
||||
ContentItemKind(MemoryContextFragment::READ_INSTRUCTIONS_KIND.to_string()),
|
||||
)
|
||||
})
|
||||
.collect()
|
||||
|
||||
@@ -13,7 +13,9 @@ use std::io;
|
||||
use std::io::Write;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_context_fragments::COMPACTION_SUMMARY_KIND;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::InternalChatMessageMetadataPassthrough;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::models::executed_tool_call_metadata_bytes;
|
||||
|
||||
@@ -68,9 +70,18 @@ impl TranscriptHistory {
|
||||
| ResponseItem::ContextCompaction { .. }
|
||||
| ResponseItem::CompactionTrigger { .. }
|
||||
)
|
||||
|| matches!(item, ResponseItem::Message { internal_chat_message_metadata_passthrough: Some(metadata), .. }
|
||||
if metadata.content_item_kinds.as_ref().is_some_and(|kinds|
|
||||
kinds.iter().any(|kind| kind.0 == "compaction.summary")))
|
||||
|| matches!(
|
||||
item,
|
||||
ResponseItem::Message {
|
||||
internal_chat_message_metadata_passthrough: Some(
|
||||
InternalChatMessageMetadataPassthrough {
|
||||
content_item_kinds: Some(kinds),
|
||||
..
|
||||
}
|
||||
),
|
||||
..
|
||||
} if kinds.iter().any(|kind| kind.as_str() == COMPACTION_SUMMARY_KIND)
|
||||
)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -20,6 +20,23 @@ fn tool(text: &str) -> ResponseItem {
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn compaction_summary_is_excluded_without_dropping_user_messages() {
|
||||
let before = message("Earlier user message");
|
||||
let summary: ResponseItem = serde_json::from_value(json!({
|
||||
"type": "message", "role": "user",
|
||||
"content": [{"type": "input_text", "text": "Summary text"}],
|
||||
"internal_chat_message_metadata_passthrough": {
|
||||
"content_item_kinds": [COMPACTION_SUMMARY_KIND]
|
||||
}
|
||||
}))
|
||||
.unwrap();
|
||||
let after = message("Summary text");
|
||||
let mut history = TranscriptHistory::default();
|
||||
history.reset([before.clone().into(), summary.into(), after.clone().into()].iter());
|
||||
assert_eq!(history.items().collect::<Vec<_>>(), vec![&before, &after]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rollback_keeps_the_earlier_prefix_or_clears_an_evicted_boundary() {
|
||||
let items = [message("keep"), message("roll back"), tool("later")];
|
||||
|
||||
@@ -32,9 +32,14 @@ pub enum MultiAgentRoleInstructions {
|
||||
},
|
||||
}
|
||||
|
||||
impl MultiAgentRoleInstructions {
|
||||
/// Content classification shared by role producers and inherited-history filtering.
|
||||
pub const KIND: &str = "multi_agent.role_instructions";
|
||||
}
|
||||
|
||||
impl ContextualUserFragment for MultiAgentRoleInstructions {
|
||||
fn content_kind(&self) -> ContentItemKind {
|
||||
ContentItemKind("multi_agent.role_instructions".to_string())
|
||||
ContentItemKind(Self::KIND.to_string())
|
||||
}
|
||||
|
||||
fn role(&self) -> &'static str {
|
||||
|
||||
Reference in New Issue
Block a user