mirror of
https://github.com/openai/codex.git
synced 2026-11-09 08:40:33 +00:00
Retire Friendly and Pragmatic personality selection (#44946)
## What changed - Use literal model instruction templates and the standard fallback prompt, ignoring legacy personality variables. Retain catalog decoding compatibility and report `supports_personality` as `false` for generated model presets. - Stop emitting `<personality_spec>` developer messages and assigning an implicit Pragmatic configuration default. - Filter Friendly/Pragmatic overrides from TUI requests while preserving explicit `personality = "none"`. With `features.personality` enabled, this opt-out still strips the model's personality section. - Refresh inherited model instructions for agent roles only when the personality opt-out changes, preserving custom instructions. ## Testing Update coverage for literal legacy templates, deprecated overrides on turn start and resume, role instruction refresh, and forwarding the explicit opt-out through thread start, resume, and fork requests. GitOrigin-RevId: 1391db4e565e010569f0885f80013d1f86b45825
This commit is contained in:
1 parent
cebdb732ea
commit
132c739171
23 files changed
+213
-1117
No files matched your search
@@ -16,7 +16,6 @@ use codex_protocol::models::BaseInstructionsProvenance;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::openai_models::AutoReviewMessages;
|
||||
use codex_protocol::openai_models::GuardianV2ModelConfig;
|
||||
use codex_protocol::openai_models::ModelInstructionsVariables;
|
||||
use codex_protocol::openai_models::ModelsResponse;
|
||||
use codex_protocol::openai_models::TruncationPolicyConfig;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
@@ -407,7 +406,7 @@ fn collaboration_replacement_wins_and_effort_clear_remains_sparse() {
|
||||
async fn model_resolution_preserves_startup_overrides_and_instruction_provenance(
|
||||
provenance: BaseInstructionsProvenance,
|
||||
) {
|
||||
let configured_instructions = "explicit {{ personality }}";
|
||||
let configured_instructions = "explicit instructions";
|
||||
let auto_review = AutoReviewMessages {
|
||||
policy: Some("catalog review policy".to_string()),
|
||||
policy_template: Some("catalog review template".to_string()),
|
||||
@@ -430,12 +429,8 @@ async fn model_resolution_preserves_startup_overrides_and_instruction_provenance
|
||||
.as_mut()
|
||||
.expect("test model should have instruction metadata");
|
||||
messages.instructions_template =
|
||||
Some("Catalog B.\n# Personality\n{{ personality }}\n# Rules\nKeep the rules.".to_string());
|
||||
messages.instructions_variables = Some(ModelInstructionsVariables {
|
||||
personality_default: Some("default".to_string()),
|
||||
personality_friendly: Some("friendly".to_string()),
|
||||
personality_pragmatic: Some("pragmatic".to_string()),
|
||||
});
|
||||
Some("Catalog B.\n# Personality\nfixed\n# Rules\nKeep the rules.".to_string());
|
||||
messages.instructions_variables = None;
|
||||
messages.auto_review = Some(auto_review);
|
||||
messages.guardian_v2 = Some(guardian_v2);
|
||||
let catalog = ModelsResponse {
|
||||
@@ -460,7 +455,7 @@ async fn model_resolution_preserves_startup_overrides_and_instruction_provenance
|
||||
(
|
||||
Personality::Friendly,
|
||||
true,
|
||||
"Catalog B.\n# Personality\nfriendly\n# Rules\nKeep the rules.",
|
||||
"Catalog B.\n# Personality\nfixed\n# Rules\nKeep the rules.",
|
||||
),
|
||||
(
|
||||
Personality::None,
|
||||
@@ -470,7 +465,7 @@ async fn model_resolution_preserves_startup_overrides_and_instruction_provenance
|
||||
(
|
||||
Personality::Friendly,
|
||||
false,
|
||||
"Catalog B.\n# Personality\ndefault\n# Rules\nKeep the rules.",
|
||||
"Catalog B.\n# Personality\nfixed\n# Rules\nKeep the rules.",
|
||||
),
|
||||
] {
|
||||
config.personality = Some(personality);
|
||||
|
||||
@@ -76,7 +76,6 @@ use codex_protocol::models::FunctionCallOutputPayload;
|
||||
use codex_protocol::models::ImageDetail;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::models::SandboxEnforcement;
|
||||
use codex_protocol::openai_models::ModelInstructionsVariables;
|
||||
use codex_protocol::openai_models::ModelServiceTier;
|
||||
use codex_protocol::openai_models::ToolMode;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
@@ -10130,9 +10129,7 @@ async fn build_initial_context_uses_retained_step_after_model_change() {
|
||||
CodexAuth::from_api_key("Test API Key"),
|
||||
Vec::new(),
|
||||
|config| {
|
||||
config.features.enable(Feature::Personality).unwrap();
|
||||
config.features.enable(Feature::TokenBudget).unwrap();
|
||||
config.personality = Some(Personality::Friendly);
|
||||
},
|
||||
)
|
||||
.await;
|
||||
@@ -10153,12 +10150,8 @@ async fn build_initial_context_uses_retained_step_after_model_change() {
|
||||
model_info.context_window = None;
|
||||
model_info.max_context_window = None;
|
||||
let messages = model_info.model_messages.as_mut().unwrap();
|
||||
messages.instructions_template = Some("A instructions: {{ personality }}".to_string());
|
||||
messages.instructions_variables = Some(ModelInstructionsVariables {
|
||||
personality_default: Some("default".to_string()),
|
||||
personality_friendly: Some("friendly".to_string()),
|
||||
personality_pragmatic: Some("pragmatic".to_string()),
|
||||
});
|
||||
messages.instructions_template = Some("A instructions".to_string());
|
||||
messages.instructions_variables = None;
|
||||
});
|
||||
session
|
||||
.set_previous_turn_settings(Some(PreviousTurnSettings {
|
||||
@@ -10181,7 +10174,6 @@ async fn build_initial_context_uses_retained_step_after_model_change() {
|
||||
|
||||
let mut selected_b = step_a.settings.selected().clone();
|
||||
selected_b.collaboration_mode.settings.model = "model-b".to_string();
|
||||
selected_b.personality = Some(Personality::Pragmatic);
|
||||
let mut model_b = step_a.settings.model_info.as_ref().clone();
|
||||
model_b.slug = "model-b".to_string();
|
||||
model_b.context_window = Some(128_000);
|
||||
@@ -10190,7 +10182,7 @@ async fn build_initial_context_uses_retained_step_after_model_change() {
|
||||
.model_messages
|
||||
.as_mut()
|
||||
.unwrap()
|
||||
.instructions_template = Some("B instructions: {{ personality }}".to_string());
|
||||
.instructions_template = Some("B instructions".to_string());
|
||||
turn_context
|
||||
.current_settings
|
||||
.store(Arc::new(ResolvedStepSettings::new(
|
||||
@@ -10218,9 +10210,9 @@ async fn build_initial_context_uses_retained_step_after_model_change() {
|
||||
.collect::<Vec<_>>();
|
||||
let a_text = developer_input_texts(&initial_a).join("\n");
|
||||
let b_text = developer_input_texts(&initial_b).join("\n");
|
||||
assert!(a_text.contains("A instructions: friendly"));
|
||||
assert!(a_text.contains("A instructions"));
|
||||
assert!(!a_text.contains("<context_window>"));
|
||||
assert!(b_text.contains("B instructions: pragmatic"));
|
||||
assert!(b_text.contains("B instructions"));
|
||||
assert!(!b_text.contains("A instructions:"));
|
||||
assert!(!a_text.contains("turn context extension enabled"));
|
||||
assert!(b_text.contains("turn context extension enabled"));
|
||||
@@ -11944,36 +11936,9 @@ async fn sample_rollout(
|
||||
let mut rollout_items = Vec::new();
|
||||
let mut live_history = ContextManager::new();
|
||||
|
||||
// Use the same turn_context source as record_initial_history so model_info (and thus
|
||||
// personality_spec) matches reconstruction.
|
||||
// Use the same turn_context source as record_initial_history so model_info matches reconstruction.
|
||||
let reconstruction_turn = session.new_default_turn().await;
|
||||
let mut initial_context = build_initial_context(session, &reconstruction_turn).await;
|
||||
// Ensure personality_spec is present when Personality is enabled, so expected matches
|
||||
// what reconstruction produces (build_initial_context may omit it when baked into model).
|
||||
if !initial_context.iter().any(|m| {
|
||||
matches!(m, ResponseItem::Message { role, content, .. }
|
||||
if role == "developer"
|
||||
&& content.iter().any(|c| {
|
||||
matches!(c, ContentItem::InputText { text } if text.contains("<personality_spec>"))
|
||||
}))
|
||||
}) && let Some(p) = reconstruction_turn.personality()
|
||||
&& session.features.enabled(Feature::Personality)
|
||||
&& let Some(personality_message) = reconstruction_turn
|
||||
.model_info()
|
||||
.model_messages
|
||||
.as_ref()
|
||||
.and_then(|m| m.get_personality_message(Some(p)).filter(|s| !s.is_empty()))
|
||||
{
|
||||
let msg = crate::context::ContextualUserFragment::into(
|
||||
crate::context::PersonalitySpecInstructions::new(personality_message),
|
||||
);
|
||||
let insert_at = initial_context
|
||||
.iter()
|
||||
.position(|m| matches!(m, ResponseItem::Message { role, .. } if role == "developer"))
|
||||
.map(|i| i + 1)
|
||||
.unwrap_or(0);
|
||||
initial_context.insert(insert_at, msg);
|
||||
}
|
||||
let initial_context = build_initial_context(session, &reconstruction_turn).await;
|
||||
for item in &initial_context {
|
||||
rollout_items.push(RolloutItem::ResponseItem(item.clone().into()));
|
||||
}
|
||||
|
||||
@@ -18,7 +18,6 @@ use crate::context::world_state::MultiAgentModeState;
|
||||
use crate::context::world_state::MultiAgentUsageHintState;
|
||||
use crate::context::world_state::PermissionsState;
|
||||
use crate::context::world_state::PersistentModeState;
|
||||
use crate::context::world_state::PersonalityState;
|
||||
use crate::context::world_state::PluginsInstructionsState;
|
||||
use crate::context::world_state::RealtimeState;
|
||||
use crate::context::world_state::ToolsState;
|
||||
@@ -57,29 +56,23 @@ impl Session {
|
||||
model_instructions
|
||||
};
|
||||
let base_instructions = self.get_prompt_base_instructions().await.text;
|
||||
let (previous_model, previous_context, base_instructions) = {
|
||||
let previous_model = {
|
||||
let state = self.state.lock().await;
|
||||
(
|
||||
state
|
||||
.previous_turn_settings()
|
||||
.map(|previous| previous.model)
|
||||
.or_else(|| {
|
||||
state
|
||||
.base_instructions_provenance
|
||||
.as_ref()
|
||||
.and_then(|provenance| match provenance {
|
||||
BaseInstructionsProvenance::Model { model } => Some(model),
|
||||
BaseInstructionsProvenance::Custom => None,
|
||||
})
|
||||
.filter(|_| base_instructions != model_instructions)
|
||||
.cloned()
|
||||
}),
|
||||
state.reference_context_item(),
|
||||
base_instructions,
|
||||
)
|
||||
state
|
||||
.previous_turn_settings()
|
||||
.map(|previous| previous.model)
|
||||
.or_else(|| {
|
||||
state
|
||||
.base_instructions_provenance
|
||||
.as_ref()
|
||||
.and_then(|provenance| match provenance {
|
||||
BaseInstructionsProvenance::Model { model } => Some(model),
|
||||
BaseInstructionsProvenance::Custom => None,
|
||||
})
|
||||
.filter(|_| base_instructions != model_instructions)
|
||||
.cloned()
|
||||
})
|
||||
};
|
||||
let personality_is_baked =
|
||||
model_info.supports_personality() && base_instructions == model_instructions;
|
||||
let environment_subagents = if turn_context.config.include_environment_context {
|
||||
self.services
|
||||
.agent_control
|
||||
@@ -97,28 +90,6 @@ impl Session {
|
||||
previous_model.as_deref(),
|
||||
model_instructions,
|
||||
));
|
||||
if self.features.enabled(Feature::Personality) {
|
||||
let personality_instructions = personality.and_then(|personality| {
|
||||
model_info
|
||||
.model_messages
|
||||
.as_ref()
|
||||
.and_then(|messages| messages.get_personality_message(Some(personality)))
|
||||
.filter(|message| !message.is_empty())
|
||||
});
|
||||
world_state.add_section(PersonalityState::new(
|
||||
&model_info.slug,
|
||||
personality,
|
||||
previous_context
|
||||
.as_ref()
|
||||
.map(|previous| previous.model.as_str())
|
||||
.or(previous_model.as_deref()),
|
||||
previous_context
|
||||
.as_ref()
|
||||
.and_then(|previous| previous.personality),
|
||||
personality_instructions,
|
||||
personality_is_baked,
|
||||
));
|
||||
}
|
||||
let token_budget_enabled = turn_context.config.features.enabled(Feature::TokenBudget)
|
||||
&& step_context
|
||||
.settings
|
||||
|
||||
Reference in new issue
Block a user