Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions codex-rs/context-fragments/src/fragment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ impl<T: ContextualUserFragment> FragmentRegistration for FragmentRegistrationPro
pub trait ContextualUserFragment {
fn role(&self) -> &'static str;

/// Whether this fragment must be recorded as its own response item.
fn requires_separate_message(&self) -> bool {
false
}

fn markers(&self) -> (&'static str, &'static str);

fn body(&self) -> String;
Expand Down
4 changes: 4 additions & 0 deletions codex-rs/core/src/context/model_switch_instructions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ impl ContextualUserFragment for ModelSwitchInstructions {
"developer"
}

fn requires_separate_message(&self) -> bool {
true
}

fn markers(&self) -> (&'static str, &'static str) {
Self::type_markers()
}
Expand Down
4 changes: 4 additions & 0 deletions codex-rs/core/src/context/personality_spec_instructions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ impl ContextualUserFragment for PersonalitySpecInstructions {
"developer"
}

fn requires_separate_message(&self) -> bool {
true
}

fn markers(&self) -> (&'static str, &'static str) {
Self::type_markers()
}
Expand Down
4 changes: 4 additions & 0 deletions codex-rs/core/src/context/world_state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ mod apps_instructions;
mod collaboration_mode;
mod environment;
mod environments_instructions;
mod model;
mod multi_agent_mode;
mod permissions;
mod personality;
mod plugins_instructions;
mod realtime;
#[cfg(test)]
Expand Down Expand Up @@ -32,8 +34,10 @@ pub(crate) use apps_instructions::AppsInstructionsState;
pub(crate) use collaboration_mode::CollaborationModeState;
pub(crate) use environment::EnvironmentsState;
pub(crate) use environments_instructions::EnvironmentsInstructionsState;
pub(crate) use model::ModelInstructionsState;
pub(crate) use multi_agent_mode::MultiAgentModeState;
pub(crate) use permissions::PermissionsState;
pub(crate) use personality::PersonalityState;
pub(crate) use plugins_instructions::PluginsInstructionsState;
pub(crate) use realtime::RealtimeState;
pub(crate) use tools::ToolsState;
Expand Down
65 changes: 65 additions & 0 deletions codex-rs/core/src/context/world_state/model.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
use super::PreviousSectionState;
use super::WorldStateSection;
use crate::context::ContextualUserFragment;
use crate::context::ModelSwitchInstructions;

/// Model identity and the instructions needed when that identity changes.
#[derive(Clone, Debug)]
pub(crate) struct ModelInstructionsState {
model: String,
previous_model: Option<String>,
instructions: String,
}

impl ModelInstructionsState {
pub(crate) fn new(model: &str, previous_model: Option<&str>, instructions: String) -> Self {
Self {
model: model.to_string(),
previous_model: previous_model.map(str::to_string),
instructions,
}
}
}

impl WorldStateSection for ModelInstructionsState {
const ID: &'static str = "model";
type Snapshot = String;

fn snapshot(&self) -> Self::Snapshot {
self.model.clone()
}

fn matches_legacy_fragment(role: &str, text: &str) -> bool {
role == "developer" && ModelSwitchInstructions::matches_text(text)
}

fn has_retained_fragment_matcher() -> bool {
true
}

fn matches_retained_fragment(role: &str, text: &str) -> bool {
Self::matches_legacy_fragment(role, text)
}

fn render_diff(
&self,
previous: PreviousSectionState<'_, Self::Snapshot>,
) -> Option<Box<dyn ContextualUserFragment>> {
let model_changed = match previous {
PreviousSectionState::Known(previous) => previous != &self.model,
PreviousSectionState::Unknown | PreviousSectionState::Absent => self
.previous_model
.as_deref()
.is_some_and(|previous| previous != self.model),
};

(model_changed && !self.instructions.is_empty()).then(|| {
Box::new(ModelSwitchInstructions::new(self.instructions.clone()))
as Box<dyn ContextualUserFragment>
})
}
}

#[cfg(test)]
#[path = "model_tests.rs"]
mod tests;
35 changes: 35 additions & 0 deletions codex-rs/core/src/context/world_state/model_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
use super::*;
use pretty_assertions::assert_eq;

#[test]
fn model_change_renders_when_persisted_or_inferred_from_previous_turn() {
let state = ModelInstructionsState::new("gpt-new", Some("gpt-old"), "instructions".into());
let previous = "gpt-old".to_string();

for previous in [
PreviousSectionState::Known(&previous),
PreviousSectionState::Unknown,
PreviousSectionState::Absent,
] {
assert_eq!(
state
.render_diff(previous)
.expect("model change should render")
.markers(),
ModelSwitchInstructions::type_markers()
);
}
}

#[test]
fn unchanged_model_does_not_render() {
let state = ModelInstructionsState::new("gpt-test", Some("gpt-test"), "instructions".into());
let previous = "gpt-test".to_string();

assert!(
state
.render_diff(PreviousSectionState::Known(&previous))
.is_none()
);
assert!(state.render_diff(PreviousSectionState::Absent).is_none());
}
101 changes: 101 additions & 0 deletions codex-rs/core/src/context/world_state/personality.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
use super::PreviousSectionState;
use super::WorldStateSection;
use crate::context::ContextualUserFragment;
use crate::context::PersonalitySpecInstructions;
use codex_protocol::config_types::Personality;
use serde::Deserialize;
use serde::Serialize;

/// Personality instructions currently visible to the model.
#[derive(Clone, Debug)]
pub(crate) struct PersonalityState {
snapshot: PersonalitySnapshot,
previous: Option<PersonalitySnapshot>,
instructions: Option<String>,
personality_is_baked: bool,
}

#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize)]
pub(crate) struct PersonalitySnapshot {
model: String,
#[serde(default, skip_serializing_if = "Option::is_none")]
personality: Option<Personality>,
}

impl PersonalityState {
pub(crate) fn new(
model: &str,
personality: Option<Personality>,
previous_model: Option<&str>,
previous_personality: Option<Personality>,
instructions: Option<String>,
personality_is_baked: bool,
) -> Self {
Self {
snapshot: PersonalitySnapshot {
model: model.to_string(),
personality,
},
previous: previous_model.map(|model| PersonalitySnapshot {
model: model.to_string(),
personality: previous_personality,
}),
instructions,
personality_is_baked,
}
}

fn render_change(
&self,
previous: &PersonalitySnapshot,
) -> Option<Box<dyn ContextualUserFragment>> {
(previous.model == self.snapshot.model && previous.personality != self.snapshot.personality)
.then_some(self.instructions.as_ref())
.flatten()
.map(|instructions| {
Box::new(PersonalitySpecInstructions::new(instructions.clone()))
as Box<dyn ContextualUserFragment>
})
}
}

impl WorldStateSection for PersonalityState {
const ID: &'static str = "personality";
type Snapshot = PersonalitySnapshot;

fn snapshot(&self) -> Self::Snapshot {
self.snapshot.clone()
}

fn matches_legacy_fragment(role: &str, text: &str) -> bool {
role == "developer" && PersonalitySpecInstructions::matches_text(text)
}

fn render_diff(
&self,
previous: PreviousSectionState<'_, Self::Snapshot>,
) -> Option<Box<dyn ContextualUserFragment>> {
match previous {
PreviousSectionState::Known(previous) => self.render_change(previous),
PreviousSectionState::Unknown => self
.previous
.as_ref()
.and_then(|previous| self.render_change(previous)),
PreviousSectionState::Absent => (!self.personality_is_baked
&& self
.previous
.as_ref()
.is_none_or(|previous| previous.model == self.snapshot.model))
.then_some(self.instructions.as_ref())
.flatten()
.map(|instructions| {
Box::new(PersonalitySpecInstructions::new(instructions.clone()))
as Box<dyn ContextualUserFragment>
}),
}
}
}

#[cfg(test)]
#[path = "personality_tests.rs"]
mod tests;
106 changes: 106 additions & 0 deletions codex-rs/core/src/context/world_state/personality_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
use super::*;
use crate::context::world_state::WorldState;
use pretty_assertions::assert_eq;

fn state(
model: &str,
personality: Option<Personality>,
previous: Option<(&str, Option<Personality>)>,
personality_is_baked: bool,
) -> PersonalityState {
PersonalityState::new(
model,
personality,
previous.map(|(model, _)| model),
previous.and_then(|(_, personality)| personality),
personality.map(|personality| format!("instructions for {personality:?}")),
personality_is_baked,
)
}

#[test]
fn initial_personality_renders_only_when_missing_from_base_instructions() {
let separate = state(
"gpt-test",
Some(Personality::Friendly),
/*previous*/ None,
/*personality_is_baked*/ false,
);
let baked = state(
"gpt-test",
Some(Personality::Friendly),
/*previous*/ None,
/*personality_is_baked*/ true,
);

assert_eq!(
separate
.render_diff(PreviousSectionState::Absent)
.expect("separate personality should render")
.markers(),
PersonalitySpecInstructions::type_markers()
);
assert!(baked.render_diff(PreviousSectionState::Absent).is_none());
}

#[test]
fn personality_changes_render_without_repeating_model_changes() {
let previous = PersonalitySnapshot {
model: "gpt-test".to_string(),
personality: Some(Personality::Friendly),
};
let changed = state(
"gpt-test",
Some(Personality::Pragmatic),
Some(("gpt-test", Some(Personality::Friendly))),
/*personality_is_baked*/ true,
);
let model_changed = state(
"gpt-next",
Some(Personality::Pragmatic),
Some(("gpt-test", Some(Personality::Friendly))),
/*personality_is_baked*/ true,
);

assert_eq!(
changed
.render_diff(PreviousSectionState::Known(&previous))
.expect("changed personality should render")
.markers(),
PersonalitySpecInstructions::type_markers()
);
assert!(
model_changed
.render_diff(PreviousSectionState::Known(&previous))
.is_none()
);
assert!(
model_changed
.render_diff(PreviousSectionState::Unknown)
.is_none()
);
assert!(
model_changed
.render_diff(PreviousSectionState::Absent)
.is_none()
);
}

#[test]
fn persisted_personality_does_not_require_a_retained_update() {
let state = state(
"gpt-test",
Some(Personality::Friendly),
Some(("gpt-test", Some(Personality::Friendly))),
/*personality_is_baked*/ false,
);
let mut world_state = WorldState::default();
world_state.add_section(state);
let snapshot = world_state.snapshot();

assert!(
world_state
.render_history_diff(Some(&snapshot), &[])
.is_empty()
);
}
Loading
Loading