diff --git a/AGENTS.md b/AGENTS.md index 3a287a5991..3ee7390387 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -41,6 +41,18 @@ In the codex-rs folder where the rust code lives: - When extracting code from a large module, move the related tests and module/type docs toward the new implementation so the invariants stay close to the code that owns them. +### Model-visible context fragments + +- Model-visible prompt context should go through the shared fragment abstractions described in `docs/model-visible-context.md`. +- Every named fragment type should implement `ModelVisibleContextFragment` and set `type Role`. +- If a fragment represents durable turn/session state that should be rebuilt correctly across resume/fork/compaction/backtracking, implement `ModelVisibleContextFragment::build(...)`. +- If a fragment is contextual-user, it must provide stable detection: prefer `contextual_user_markers()` when fixed markers are sufficient, and override `matches_contextual_user_text()` only for genuinely custom matching (for example AGENTS.md). +- Choose the role intentionally: developer guidance belongs in `DeveloperContextRole`; contextual user-role state belongs in `ContextualUserContextRole`. +- Use contextual-user fragments for contextual user-role state that must be parsed as context rather than literal user intent. +- Runtime/session-prefix fragments that are not turn-state diffs should usually leave `ModelVisibleContextFragment::build(...)` as `None`. +- Prefer dedicated typed fragments over plain strings. Developer-only one-off text is acceptable only when it is truly isolated, does not need contextual-user detection, and does not participate in turn-state diff reconstruction. +- Do not hand-construct model-visible `ResponseItem::Message` payloads in new turn-state code; use fragment conversion helpers from `codex-rs/core/src/model_visible_context.rs`. + Run `just fmt` (in `codex-rs` directory) automatically after you have finished making Rust code changes; do not ask for approval to run it. Additionally, run the tests: 1. Run the test for the specific project that was changed. For example, if changes were made in `codex-rs/tui`, run `cargo test -p codex-tui`. diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 29436a0d7f..d72b207d8b 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -64,6 +64,7 @@ pub mod mention_syntax; mod mentions; pub mod message_history; mod model_provider_info; +mod model_visible_context; pub mod path_utils; pub mod personality_migration; pub mod plugins; diff --git a/codex-rs/core/src/model_visible_context.rs b/codex-rs/core/src/model_visible_context.rs new file mode 100644 index 0000000000..e913ebe06b --- /dev/null +++ b/codex-rs/core/src/model_visible_context.rs @@ -0,0 +1,290 @@ +//! Shared model-visible context abstractions. +//! +//! Use this path for any injected prompt context, whether it renders in the +//! developer envelope or the contextual-user envelope. +//! +//! This module keeps only the shared rendering, role, marker, and turn-context +//! parameter helpers that fragment implementations can reuse. +//! +//! Contributor guide: +//! +//! - If the model should not see the data, do not add a fragment. +//! - If it should, prefer a typed fragment that implements +//! `ModelVisibleContextFragment`. +//! - Choose the role intentionally: +//! - `DeveloperContextRole` for developer guidance/policy +//! - `ContextualUserContextRole` for contextual user-role state that must be +//! parsed as context rather than literal user intent +//! - If the fragment is durable turn/session state that should rebuild across +//! resume, compaction, backtracking, or fork, implement `build(...)`. +//! `reference_context_item` is the baseline already represented in +//! model-visible history; compare against it to avoid duplicates, and use +//! `TurnContextDiffParams` for other runtime/session inputs such as +//! `previous_turn_settings`. +//! - If the fragment is a runtime/session-prefix marker rather than turn-state +//! context, leave `build(...)` as `None`. +//! - Contextual-user fragments must have stable detection. Prefer +//! `contextual_user_markers()`; override `matches_contextual_user_text()` +//! only when matching is genuinely custom. +//! - Keep logic fragment-local. The fragment type should own rendering, +//! state/diff inspection, and contextual-user detection when applicable. + +#![allow(dead_code)] + +use crate::codex::PreviousTurnSettings; +use crate::codex::TurnContext; +use crate::plugins::PluginCapabilitySummary; +use crate::shell::Shell; +use codex_execpolicy::Policy; +use codex_protocol::models::ContentItem; +use codex_protocol::models::ResponseInputItem; +use codex_protocol::models::ResponseItem; +use codex_protocol::protocol::TurnContextItem; + +pub(crate) const SKILL_OPEN_TAG: &str = ""; +pub(crate) const SKILL_CLOSE_TAG: &str = ""; +pub(crate) const JS_REPL_INSTRUCTIONS_OPEN_TAG: &str = ""; +pub(crate) const JS_REPL_INSTRUCTIONS_CLOSE_TAG: &str = ""; +pub(crate) const CHILD_AGENTS_INSTRUCTIONS_OPEN_TAG: &str = ""; +pub(crate) const CHILD_AGENTS_INSTRUCTIONS_CLOSE_TAG: &str = ""; +pub(crate) const USER_SHELL_COMMAND_OPEN_TAG: &str = ""; +pub(crate) const USER_SHELL_COMMAND_CLOSE_TAG: &str = ""; +pub(crate) const TURN_ABORTED_OPEN_TAG: &str = ""; +pub(crate) const TURN_ABORTED_CLOSE_TAG: &str = ""; +pub(crate) const SUBAGENTS_OPEN_TAG: &str = ""; +pub(crate) const SUBAGENTS_CLOSE_TAG: &str = ""; +pub(crate) const SUBAGENT_NOTIFICATION_OPEN_TAG: &str = ""; +pub(crate) const SUBAGENT_NOTIFICATION_CLOSE_TAG: &str = ""; + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum ModelVisibleMessageRole { + Developer, + User, +} + +impl ModelVisibleMessageRole { + const fn as_str(self) -> &'static str { + match self { + Self::Developer => "developer", + Self::User => "user", + } + } +} + +pub(crate) trait ModelVisibleContextRole { + const MESSAGE_ROLE: ModelVisibleMessageRole; +} + +pub(crate) struct DeveloperContextRole; + +impl ModelVisibleContextRole for DeveloperContextRole { + const MESSAGE_ROLE: ModelVisibleMessageRole = ModelVisibleMessageRole::Developer; +} + +pub(crate) struct ContextualUserContextRole; + +impl ModelVisibleContextRole for ContextualUserContextRole { + const MESSAGE_ROLE: ModelVisibleMessageRole = ModelVisibleMessageRole::User; +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct ContextualUserFragmentMarkers { + start_marker: &'static str, + end_marker: &'static str, +} + +impl ContextualUserFragmentMarkers { + pub(crate) const fn new(start_marker: &'static str, end_marker: &'static str) -> Self { + Self { + start_marker, + end_marker, + } + } + + pub(crate) fn matches_text(self, text: &str) -> bool { + let trimmed = text.trim_start(); + let starts_with_marker = trimmed + .get(..self.start_marker.len()) + .is_some_and(|candidate| candidate.eq_ignore_ascii_case(self.start_marker)); + let trimmed = trimmed.trim_end(); + let ends_with_marker = trimmed + .get(trimmed.len().saturating_sub(self.end_marker.len())..) + .is_some_and(|candidate| candidate.eq_ignore_ascii_case(self.end_marker)); + starts_with_marker && ends_with_marker + } + + pub(crate) fn wrap_body(self, body: String) -> String { + format!("{}\n{}\n{}", self.start_marker, body, self.end_marker) + } +} + +pub(crate) fn model_visible_content_item(text: String) -> ContentItem { + ContentItem::InputText { text } +} + +pub(crate) fn model_visible_message(text: String) -> ResponseItem { + ResponseItem::Message { + id: None, + role: R::MESSAGE_ROLE.as_str().to_owned(), + content: vec![model_visible_content_item(text)], + end_turn: None, + phase: None, + } +} + +pub(crate) fn model_visible_response_input_item( + text: String, +) -> ResponseInputItem { + ResponseInputItem::Message { + role: R::MESSAGE_ROLE.as_str().to_owned(), + content: vec![model_visible_content_item(text)], + } +} + +pub(crate) struct TurnContextDiffParams<'a> { + pub(crate) shell: &'a Shell, + pub(crate) previous_turn_settings: Option<&'a PreviousTurnSettings>, + pub(crate) exec_policy: &'a Policy, + pub(crate) personality_feature_enabled: bool, + pub(crate) base_instructions: Option<&'a str>, + pub(crate) plugin_capability_summaries: Option<&'a [PluginCapabilitySummary]>, +} + +impl<'a> TurnContextDiffParams<'a> { + pub(crate) fn new( + shell: &'a Shell, + previous_turn_settings: Option<&'a PreviousTurnSettings>, + exec_policy: &'a Policy, + personality_feature_enabled: bool, + base_instructions: Option<&'a str>, + plugin_capability_summaries: Option<&'a [PluginCapabilitySummary]>, + ) -> Self { + Self { + shell, + previous_turn_settings, + exec_policy, + personality_feature_enabled, + base_instructions, + plugin_capability_summaries, + } + } +} + +/// Implement this for any model-visible prompt fragment, regardless of which +/// envelope it renders into. +pub(crate) trait ModelVisibleContextFragment: Sized { + type Role: ModelVisibleContextRole; + + fn render_text(&self) -> String; + + /// Build the fragment from the current turn state and an optional baseline + /// context item that represents the turn state already reflected in + /// model-visible history. + /// + /// Implementations that are not turn-state fragments should leave the + /// default `None`. + fn build( + _turn_context: &TurnContext, + _reference_context_item: Option<&TurnContextItem>, + _params: &TurnContextDiffParams<'_>, + ) -> Option { + None + } + + /// Stable markers used to recognize contextual-user fragments in persisted + /// history. Developer fragments should keep the default `None`. + fn contextual_user_markers() -> Option { + None + } + + fn matches_contextual_user_text(text: &str) -> bool { + Self::contextual_user_markers().is_some_and(|markers| markers.matches_text(text)) + } + + fn wrap_contextual_user_body(body: String) -> String { + let Some(markers) = Self::contextual_user_markers() else { + panic!("contextual-user fragments using wrap_contextual_user_body must define markers"); + }; + markers.wrap_body(body) + } + + fn into_content_item(self) -> ContentItem { + model_visible_content_item(self.render_text()) + } + + fn into_message(self) -> ResponseItem { + model_visible_message::(self.render_text()) + } + + fn into_response_input_item(self) -> ResponseInputItem { + model_visible_response_input_item::(self.render_text()) + } +} + +pub(crate) struct DeveloperTextFragment { + text: String, +} + +impl DeveloperTextFragment { + pub(crate) fn new(text: impl Into) -> Self { + Self { text: text.into() } + } +} + +pub(crate) struct ContextualUserTextFragment { + text: String, +} + +impl ContextualUserTextFragment { + pub(crate) fn new(text: impl Into) -> Self { + Self { text: text.into() } + } +} + +impl ModelVisibleContextFragment for DeveloperTextFragment { + type Role = DeveloperContextRole; + + fn render_text(&self) -> String { + self.text.clone() + } +} + +impl ModelVisibleContextFragment for ContextualUserTextFragment { + type Role = ContextualUserContextRole; + + fn render_text(&self) -> String { + self.text.clone() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn contextual_user_markers_match_case_insensitive_wrapped_text() { + let markers = ContextualUserFragmentMarkers::new("", ""); + let text = " \nbody\n "; + + assert_eq!(markers.matches_text(text), true); + } + + #[test] + fn developer_role_message_uses_developer_wire_role() { + let message = model_visible_message::("hi".to_owned()); + + assert_eq!( + message, + ResponseItem::Message { + id: None, + role: "developer".to_owned(), + content: vec![ContentItem::InputText { + text: "hi".to_owned() + }], + end_turn: None, + phase: None, + } + ); + } +} diff --git a/docs/model-visible-context.md b/docs/model-visible-context.md new file mode 100644 index 0000000000..07be2f14ea --- /dev/null +++ b/docs/model-visible-context.md @@ -0,0 +1,115 @@ +# Model-visible context + +This document defines the shared foundation for model-visible prompt context in +`codex-rs`. + +The goal of this layer is simple: if the model should see some structured +context, represent it as a typed fragment instead of hand-assembling ad hoc +message payloads. That keeps rendering, role choice, and contextual-user +detection consistent before higher-level assembly code starts composing larger +prompt envelopes. + +The shared foundation lives in +[`codex-rs/core/src/model_visible_context.rs`](../codex-rs/core/src/model_visible_context.rs). + +Follow-up work will build on this foundation by introducing concrete fragment +types and wiring them into turn-state assembly, history parsing, and prompt +diffing. Those integrations are intentionally out of scope for this document. + +## Core concepts + +### Fragment role + +Every named fragment type implements `ModelVisibleContextFragment` and declares +its role with `type Role`. + +Use: + +- `DeveloperContextRole` for developer guidance and policy text +- `ContextualUserContextRole` for contextual user-role state that should be + parsed as context rather than literal user intent + +Choosing the right role is part of the fragment contract, not a later callsite +decision. + +### Fragment rendering + +`ModelVisibleContextFragment` owns the text rendering for the fragment via +`render_text()`. Shared helpers convert that rendered text into the standard +model-visible payload shapes: + +- `into_content_item()` +- `into_message()` +- `into_response_input_item()` + +New model-visible context should use these conversions instead of +hand-constructing `ResponseItem::Message` payloads. + +### Turn-state rebuild hook + +Fragments that represent durable turn or session state can implement: + +`ModelVisibleContextFragment::build(...)` + +That hook receives: + +- the current `TurnContext` +- an optional `reference_context_item` +- `TurnContextDiffParams` + +The intended use is: + +- compare current state against the persisted baseline in + `reference_context_item` +- emit `Some(fragment)` only when the current prompt state needs to be injected +- leave the default `None` for fragments that are not rebuilt from turn state + +This hook exists so later integrations can rebuild context correctly across +resume, compaction, backtracking, and fork. + +### Contextual-user detection + +Contextual-user fragments share the `user` role with real user messages, so +they need stable detection. + +Preferred path: + +- implement `contextual_user_markers()` when the fragment has fixed wrappers + +Fallback path: + +- override `matches_contextual_user_text()` when matching is genuinely custom + +The foundation intentionally keeps that detection API close to fragment +definitions so later history-parsing code can rely on it consistently. + +## Included shared helpers + +The foundation module currently provides: + +- `ModelVisibleContextRole` +- `DeveloperContextRole` +- `ContextualUserContextRole` +- `ContextualUserFragmentMarkers` +- `TurnContextDiffParams` +- `DeveloperTextFragment` +- `ContextualUserTextFragment` +- shared wrapper/tag constants for current contextual marker shapes + +These helpers are intentionally generic. They should stay reusable across +future fragment definitions rather than reflecting any one prompt assembly path. + +## Contributor guidance + +When adding model-visible context: + +1. Decide whether the model should see it at all. +2. If it should, prefer a typed fragment over a plain string. +3. Choose the role intentionally. +4. If the fragment is durable turn/session state, implement `build(...)`. +5. If the fragment is contextual-user, provide stable detection. +6. Use the shared conversion helpers instead of custom message assembly. + +Developer-only one-off text can still be acceptable when it is truly isolated +and does not need contextual-user detection or turn-state reconstruction, but +that should be the exception rather than the default.