mirror of
https://github.com/openai/codex.git
synced 2026-10-29 19:08:43 +00:00
Unify contextual user fragment detection
Collapse the contextual-user detection APIs into a single ContextualUserFragment trait with default marker-based matching and wrapping. Update the fragment docs and registry naming to match the new blessed path, and fix the stale arc monitor test reference exposed by the refactor. Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
co-authored by
Codex
parent
4f554b902a
commit
fff5f28ce1
@@ -41,7 +41,7 @@ In the codex-rs folder where the rust code lives:
|
||||
- Model-visible prompt context should go through the shared fragment abstractions described in `docs/model-visible-context.md`.
|
||||
- Every new model-visible fragment should implement `ModelVisibleContextFragment` and set `type Role`.
|
||||
- Turn-state model-visible context assembly should produce exactly two envelopes (one developer message + one contextual-user message) via the shared envelope builders.
|
||||
- Contextual-user fragments should use the shared detection path in `model_visible_context`: implement `TaggedContextualUserFragment` when marker-based detection is sufficient, or implement `ContextualUserFragmentDetector` for custom matching (for example AGENTS.md dynamic tags).
|
||||
- Contextual-user fragments should use the shared detection path in `model_visible_context`: implement `ContextualUserFragment`, prefer defining `markers()` when marker-based detection/wrapping is sufficient, and override `matches_contextual_user_text()` only for genuinely custom matching (for example AGENTS.md).
|
||||
- Use the developer envelope for developer guidance. Custom override text (for example config/app-server `developer_instructions`) should use `CustomDeveloperInstructions`; system-generated developer context should use typed fragments plus the neutral `developer_*_text` helpers rather than reusing the custom override type.
|
||||
- Use the contextual-user envelope for user-role contextual state or runtime markers such as AGENTS instructions, plugin instructions, environment context, skills, and shell-command markers. Contextual-user fragments must provide stable markers so history parsing treats them as contextual state rather than user intent.
|
||||
- Use `<environment_context>` specifically for environment facts derived from `TurnContext` that may need turn-to-turn diffs (`cwd`, `shell`, optional `current_date`, optional `timezone`, optional network allow/deny domain summaries). Do not put policy text, plugin/skill listings, or other guidance into `<environment_context>`; those should use dedicated fragments.
|
||||
|
||||
@@ -71,11 +71,11 @@ async fn build_arc_monitor_request_includes_relevant_history_and_null_policies()
|
||||
.await;
|
||||
session
|
||||
.record_into_history(
|
||||
&[
|
||||
crate::contextual_user_message::ENVIRONMENT_CONTEXT_FRAGMENT.into_message(
|
||||
"<environment_context>\n<cwd>/tmp</cwd>\n</environment_context>".to_string(),
|
||||
),
|
||||
],
|
||||
&[crate::model_visible_context::model_visible_message::<
|
||||
crate::model_visible_context::ContextualUserContextRole,
|
||||
>(
|
||||
"<environment_context>\n<cwd>/tmp</cwd>\n</environment_context>".to_string(),
|
||||
)],
|
||||
&turn_context,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
use crate::codex::TurnContext;
|
||||
use crate::model_visible_context::ContextualUserContextRole;
|
||||
use crate::model_visible_context::ContextualUserFragment;
|
||||
use crate::model_visible_context::ContextualUserFragmentMarkers;
|
||||
use crate::model_visible_context::ModelVisibleContextFragment;
|
||||
use crate::model_visible_context::TaggedContextualUserFragment;
|
||||
use crate::model_visible_context::TurnContextDiffFragment;
|
||||
use crate::model_visible_context::TurnContextDiffParams;
|
||||
use crate::shell::Shell;
|
||||
@@ -134,8 +134,10 @@ impl ModelVisibleContextFragment for EnvironmentContext {
|
||||
}
|
||||
}
|
||||
|
||||
impl TaggedContextualUserFragment for EnvironmentContext {
|
||||
const MARKERS: ContextualUserFragmentMarkers = Self::MARKERS;
|
||||
impl ContextualUserFragment for EnvironmentContext {
|
||||
fn markers() -> Option<ContextualUserFragmentMarkers> {
|
||||
Some(Self::MARKERS)
|
||||
}
|
||||
}
|
||||
|
||||
impl TurnContextDiffFragment for EnvironmentContext {
|
||||
|
||||
@@ -9,14 +9,13 @@ use serde::Serialize;
|
||||
|
||||
use crate::codex::TurnContext;
|
||||
use crate::model_visible_context::ContextualUserContextRole;
|
||||
use crate::model_visible_context::ContextualUserFragmentDetector;
|
||||
use crate::model_visible_context::ContextualUserFragment;
|
||||
use crate::model_visible_context::ContextualUserFragmentMarkers;
|
||||
use crate::model_visible_context::ModelVisibleContextFragment;
|
||||
use crate::model_visible_context::PLUGINS_CLOSE_TAG;
|
||||
use crate::model_visible_context::PLUGINS_OPEN_TAG;
|
||||
use crate::model_visible_context::SKILL_CLOSE_TAG;
|
||||
use crate::model_visible_context::SKILL_OPEN_TAG;
|
||||
use crate::model_visible_context::TaggedContextualUserFragment;
|
||||
use crate::model_visible_context::TurnContextDiffFragment;
|
||||
use crate::model_visible_context::TurnContextDiffParams;
|
||||
use codex_protocol::protocol::TurnContextItem;
|
||||
@@ -76,7 +75,7 @@ impl TurnContextDiffFragment for AgentsMdInstructions {
|
||||
}
|
||||
}
|
||||
|
||||
impl ContextualUserFragmentDetector for AgentsMdInstructions {
|
||||
impl ContextualUserFragment for AgentsMdInstructions {
|
||||
fn matches_contextual_user_text(text: &str) -> bool {
|
||||
let trimmed = text.trim_start();
|
||||
// TODO(ccunningham): Switch detection to the XML-ish wrapper once we
|
||||
@@ -102,16 +101,20 @@ impl ModelVisibleContextFragment for SkillInstructions {
|
||||
type Role = ContextualUserContextRole;
|
||||
|
||||
fn render_text(&self) -> String {
|
||||
Self::wrap_contextual_user_body(format!(
|
||||
<Self as ContextualUserFragment>::wrap_contextual_user_body(format!(
|
||||
"<name>{}</name>\n<path>{}</path>\n{}",
|
||||
self.name, self.path, self.contents
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
impl TaggedContextualUserFragment for SkillInstructions {
|
||||
const MARKERS: ContextualUserFragmentMarkers =
|
||||
ContextualUserFragmentMarkers::new(SKILL_OPEN_TAG, SKILL_CLOSE_TAG);
|
||||
impl ContextualUserFragment for SkillInstructions {
|
||||
fn markers() -> Option<ContextualUserFragmentMarkers> {
|
||||
Some(ContextualUserFragmentMarkers::new(
|
||||
SKILL_OPEN_TAG,
|
||||
SKILL_CLOSE_TAG,
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -128,13 +131,17 @@ impl ModelVisibleContextFragment for PluginInstructions {
|
||||
type Role = ContextualUserContextRole;
|
||||
|
||||
fn render_text(&self) -> String {
|
||||
Self::wrap_contextual_user_body(self.text.clone())
|
||||
<Self as ContextualUserFragment>::wrap_contextual_user_body(self.text.clone())
|
||||
}
|
||||
}
|
||||
|
||||
impl TaggedContextualUserFragment for PluginInstructions {
|
||||
const MARKERS: ContextualUserFragmentMarkers =
|
||||
ContextualUserFragmentMarkers::new(PLUGINS_OPEN_TAG, PLUGINS_CLOSE_TAG);
|
||||
impl ContextualUserFragment for PluginInstructions {
|
||||
fn markers() -> Option<ContextualUserFragmentMarkers> {
|
||||
Some(ContextualUserFragmentMarkers::new(
|
||||
PLUGINS_OPEN_TAG,
|
||||
PLUGINS_CLOSE_TAG,
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -181,7 +188,7 @@ mod tests {
|
||||
}
|
||||
));
|
||||
assert!(
|
||||
<AgentsMdInstructions as ContextualUserFragmentDetector>::matches_contextual_user_text(
|
||||
<AgentsMdInstructions as ContextualUserFragment>::matches_contextual_user_text(
|
||||
"# AGENTS.md instructions for test_directory\n\n<INSTRUCTIONS>\ntest_text\n</INSTRUCTIONS>"
|
||||
)
|
||||
);
|
||||
@@ -215,12 +222,12 @@ mod tests {
|
||||
#[test]
|
||||
fn test_is_skill_instructions() {
|
||||
assert!(
|
||||
<SkillInstructions as ContextualUserFragmentDetector>::matches_contextual_user_text(
|
||||
<SkillInstructions as ContextualUserFragment>::matches_contextual_user_text(
|
||||
"<skill>\n<name>demo-skill</name>\n<path>skills/demo/SKILL.md</path>\nbody\n</skill>"
|
||||
)
|
||||
);
|
||||
assert!(
|
||||
!<SkillInstructions as ContextualUserFragmentDetector>::matches_contextual_user_text(
|
||||
!<SkillInstructions as ContextualUserFragment>::matches_contextual_user_text(
|
||||
"regular text"
|
||||
)
|
||||
);
|
||||
|
||||
@@ -53,21 +53,20 @@ pub(crate) struct ContextualUserFragmentMarkers {
|
||||
end_marker: &'static str,
|
||||
}
|
||||
|
||||
pub(crate) trait ContextualUserFragmentDetector {
|
||||
fn matches_contextual_user_text(text: &str) -> bool;
|
||||
}
|
||||
pub(crate) trait ContextualUserFragment {
|
||||
fn markers() -> Option<ContextualUserFragmentMarkers> {
|
||||
None
|
||||
}
|
||||
|
||||
pub(crate) trait TaggedContextualUserFragment {
|
||||
const MARKERS: ContextualUserFragmentMarkers;
|
||||
fn matches_contextual_user_text(text: &str) -> bool {
|
||||
Self::markers().is_some_and(|markers| markers.matches_text(text))
|
||||
}
|
||||
|
||||
fn wrap_contextual_user_body(body: String) -> String {
|
||||
Self::MARKERS.wrap_body(body)
|
||||
}
|
||||
}
|
||||
|
||||
impl<T: TaggedContextualUserFragment> ContextualUserFragmentDetector for T {
|
||||
fn matches_contextual_user_text(text: &str) -> bool {
|
||||
T::MARKERS.matches_text(text)
|
||||
let markers = Self::markers().expect(
|
||||
"contextual-user fragments using wrap_contextual_user_body must define markers",
|
||||
);
|
||||
markers.wrap_body(body)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -241,7 +240,7 @@ pub(crate) trait TurnContextDiffFragment: ModelVisibleContextFragment + Sized {
|
||||
) -> Option<Self>;
|
||||
}
|
||||
|
||||
fn detect_contextual_user_fragment<F: ContextualUserFragmentDetector>(text: &str) -> bool {
|
||||
fn detect_contextual_user_fragment<F: ContextualUserFragment>(text: &str) -> bool {
|
||||
F::matches_contextual_user_text(text)
|
||||
}
|
||||
|
||||
@@ -251,7 +250,7 @@ fn build_contextual_user_turn_state_fragment<F>(
|
||||
params: &TurnContextDiffParams<'_>,
|
||||
) -> Option<ContextualUserTextFragment>
|
||||
where
|
||||
F: TurnContextDiffFragment<Role = ContextualUserContextRole>,
|
||||
F: TurnContextDiffFragment<Role = ContextualUserContextRole> + ContextualUserFragment,
|
||||
{
|
||||
let fragment = F::build(turn_context, reference_context_item, params)?;
|
||||
Some(ContextualUserTextFragment::new(fragment.render_text()))
|
||||
@@ -263,8 +262,9 @@ where
|
||||
/// 1. Defining a typed fragment struct.
|
||||
/// 2. Implementing `ModelVisibleContextFragment` with
|
||||
/// `Role = ContextualUserContextRole`.
|
||||
/// 3. Implementing contextual-user detection via
|
||||
/// `TaggedContextualUserFragment` or `ContextualUserFragmentDetector`.
|
||||
/// 3. Implementing `ContextualUserFragment` for detection. Prefer defining
|
||||
/// `markers()` so the default matcher/wrapper behavior applies; override
|
||||
/// `matches_contextual_user_text()` only for genuinely custom formats.
|
||||
/// 4. If the fragment is derived from turn state, implementing
|
||||
/// `TurnContextDiffFragment::build` and registering it with
|
||||
/// `Some(build_contextual_user_turn_state_fragment::<YourType>)`.
|
||||
@@ -403,7 +403,7 @@ mod tests {
|
||||
#[test]
|
||||
fn marker_matching_ignores_plain_text() {
|
||||
assert!(
|
||||
!<crate::instructions::SkillInstructions as ContextualUserFragmentDetector>::matches_contextual_user_text("plain text")
|
||||
!<crate::instructions::SkillInstructions as ContextualUserFragment>::matches_contextual_user_text("plain text")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -24,11 +24,11 @@ use crate::codex::Session;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::event_mapping::parse_turn_item;
|
||||
use crate::model_visible_context::ContextualUserContextRole;
|
||||
use crate::model_visible_context::ContextualUserFragment;
|
||||
use crate::model_visible_context::ContextualUserFragmentMarkers;
|
||||
use crate::model_visible_context::ModelVisibleContextFragment;
|
||||
use crate::model_visible_context::TURN_ABORTED_CLOSE_TAG;
|
||||
use crate::model_visible_context::TURN_ABORTED_OPEN_TAG;
|
||||
use crate::model_visible_context::TaggedContextualUserFragment;
|
||||
use crate::models_manager::manager::ModelsManager;
|
||||
use crate::protocol::EventMsg;
|
||||
use crate::protocol::TokenUsage;
|
||||
@@ -70,13 +70,17 @@ impl ModelVisibleContextFragment for TurnAbortedMarker {
|
||||
type Role = ContextualUserContextRole;
|
||||
|
||||
fn render_text(&self) -> String {
|
||||
Self::wrap_contextual_user_body(self.guidance.to_string())
|
||||
<Self as ContextualUserFragment>::wrap_contextual_user_body(self.guidance.to_string())
|
||||
}
|
||||
}
|
||||
|
||||
impl TaggedContextualUserFragment for TurnAbortedMarker {
|
||||
const MARKERS: ContextualUserFragmentMarkers =
|
||||
ContextualUserFragmentMarkers::new(TURN_ABORTED_OPEN_TAG, TURN_ABORTED_CLOSE_TAG);
|
||||
impl ContextualUserFragment for TurnAbortedMarker {
|
||||
fn markers() -> Option<ContextualUserFragmentMarkers> {
|
||||
Some(ContextualUserFragmentMarkers::new(
|
||||
TURN_ABORTED_OPEN_TAG,
|
||||
TURN_ABORTED_CLOSE_TAG,
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
fn emit_turn_network_proxy_metric(
|
||||
|
||||
@@ -5,9 +5,9 @@ use codex_protocol::models::ResponseItem;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::exec::ExecToolCallOutput;
|
||||
use crate::model_visible_context::ContextualUserContextRole;
|
||||
use crate::model_visible_context::ContextualUserFragment;
|
||||
use crate::model_visible_context::ContextualUserFragmentMarkers;
|
||||
use crate::model_visible_context::ModelVisibleContextFragment;
|
||||
use crate::model_visible_context::TaggedContextualUserFragment;
|
||||
use crate::model_visible_context::USER_SHELL_COMMAND_CLOSE_TAG;
|
||||
use crate::model_visible_context::USER_SHELL_COMMAND_OPEN_TAG;
|
||||
use crate::tools::format_exec_output_str;
|
||||
@@ -19,11 +19,13 @@ fn format_duration_line(duration: Duration) -> String {
|
||||
|
||||
pub(crate) struct UserShellCommandFragment;
|
||||
|
||||
impl TaggedContextualUserFragment for UserShellCommandFragment {
|
||||
const MARKERS: ContextualUserFragmentMarkers = ContextualUserFragmentMarkers::new(
|
||||
USER_SHELL_COMMAND_OPEN_TAG,
|
||||
USER_SHELL_COMMAND_CLOSE_TAG,
|
||||
);
|
||||
impl ContextualUserFragment for UserShellCommandFragment {
|
||||
fn markers() -> Option<ContextualUserFragmentMarkers> {
|
||||
Some(ContextualUserFragmentMarkers::new(
|
||||
USER_SHELL_COMMAND_OPEN_TAG,
|
||||
USER_SHELL_COMMAND_CLOSE_TAG,
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
struct UserShellCommandRecord<'a> {
|
||||
@@ -49,7 +51,9 @@ impl ModelVisibleContextFragment for UserShellCommandRecord<'_> {
|
||||
self.turn_context.truncation_policy,
|
||||
));
|
||||
sections.push("</result>".to_string());
|
||||
UserShellCommandFragment::wrap_contextual_user_body(sections.join("\n"))
|
||||
<UserShellCommandFragment as ContextualUserFragment>::wrap_contextual_user_body(
|
||||
sections.join("\n"),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -7,10 +7,10 @@ use pretty_assertions::assert_eq;
|
||||
#[test]
|
||||
fn detects_user_shell_command_text_variants() {
|
||||
assert!(
|
||||
<UserShellCommandFragment as crate::model_visible_context::ContextualUserFragmentDetector>::matches_contextual_user_text("<user_shell_command>\necho hi\n</user_shell_command>")
|
||||
<UserShellCommandFragment as crate::model_visible_context::ContextualUserFragment>::matches_contextual_user_text("<user_shell_command>\necho hi\n</user_shell_command>")
|
||||
);
|
||||
assert!(
|
||||
!<UserShellCommandFragment as crate::model_visible_context::ContextualUserFragmentDetector>::matches_contextual_user_text("echo hi")
|
||||
!<UserShellCommandFragment as crate::model_visible_context::ContextualUserFragment>::matches_contextual_user_text("echo hi")
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -26,11 +26,12 @@ When adding new model-visible context:
|
||||
2. Implement `ModelVisibleContextFragment` for it.
|
||||
3. Set the fragment `type Role` to the correct developer or contextual-user role.
|
||||
4. If it is a contextual-user fragment, implement contextual-user detection:
|
||||
- prefer `TaggedContextualUserFragment` for marker-based detection/wrapping
|
||||
- use `ContextualUserFragmentDetector` when matching is dynamic (for example AGENTS.md tags that embed directory names)
|
||||
- implement `ContextualUserFragment`
|
||||
- prefer defining `markers()` so the default marker-based detection/wrapping applies
|
||||
- override `matches_contextual_user_text()` only when matching is genuinely custom (for example AGENTS.md)
|
||||
5. If the fragment is derived from durable/current turn state and should be diffed/reinjected after history mutations, also implement `TurnContextDiffFragment`.
|
||||
6. Register the fragment type:
|
||||
- developer turn-state fragments: add to `REGISTERED_DEVELOPER_TURN_STATE_FRAGMENT_BUILDERS` in `context_manager/updates.rs`.
|
||||
- developer turn-state fragments: add to `REGISTERED_TURN_STATE_FRAGMENT_BUILDERS` in `context_manager/updates.rs`.
|
||||
- contextual-user fragments: add once to `REGISTERED_CONTEXTUAL_USER_FRAGMENTS` in `model_visible_context.rs` (this powers both history detection and optional turn-state diff assembly).
|
||||
7. Push the resulting fragments through the shared envelope builders.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user