mirror of
https://github.com/openai/codex.git
synced 2026-11-13 21:30:47 +00:00
Track model and personality in world state (#35530)
## What changed - Add model and personality sections to the persisted world-state snapshot. - Generate model-switch and personality instructions through world-state diffs, including when prior state must be inferred during replay. - Keep those instruction fragments in standalone developer messages instead of merging them with adjacent context updates. ## Testing - Cover initial, unchanged, and changed model and personality states. - Verify that model changes are recorded in rollout world-state items. GitOrigin-RevId: 497e29cf38cac08e992e546df900f6f8b01120f9
This commit is contained in:
@@ -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()
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
|
||||
@@ -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)]
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
@@ -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());
|
||||
}
|
||||
@@ -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;
|
||||
@@ -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()
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user