mirror of
https://github.com/openai/codex.git
synced 2026-11-09 00:39:32 +00:00
Use captured step settings when building model context (#44200)
## Why Model changes during an active turn could leave context using the turn's initial model and settings, even after requests switched to the new model. ## What changed - Build model, personality, approval, collaboration, app, plugin, and multi-agent instructions from captured step settings. - Resolve multi-agent policy from the step's effective reasoning effort. - Use the captured step's context window for full-context metadata and retain that step when rebuilding initial context for compaction. ## Testing Add regression coverage for model switches updating context without duplicating updates, delegation policy following selected or model-default reasoning effort, and compaction preserving retained context after a model change. GitOrigin-RevId: 3615c7a3f13f15fdcce9ad13d23311340e0aea88
This commit is contained in:
1 parent
ccf470c060
commit
b4507997e0
11 files changed
+373
-69
No files matched your search
@@ -3977,11 +3977,15 @@ impl Session {
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// `step_context` and `world_state` must come from the same captured step.
|
||||
/// If more callers need this pair, bundle them into a captured-context struct
|
||||
/// so callers cannot mix settings and WorldState from different steps.
|
||||
pub(crate) async fn build_initial_context_with_world_state(
|
||||
&self,
|
||||
turn_context: &TurnContext,
|
||||
step_context: &StepContext,
|
||||
world_state: &WorldState,
|
||||
) -> Vec<ResponseItem> {
|
||||
let turn_context = step_context.turn.as_ref();
|
||||
let mut developer_sections = Vec::<RenderedFragment>::with_capacity(8);
|
||||
let mut contextual_user_sections = Vec::<RenderedFragment>::with_capacity(2);
|
||||
let mut separate_developer_sections = Vec::<RenderedFragment>::new();
|
||||
@@ -4071,7 +4075,11 @@ impl Session {
|
||||
}
|
||||
// This is full-context metadata. Steady-state context diffs should not re-emit it.
|
||||
if turn_context.config.features.enabled(Feature::TokenBudget)
|
||||
&& turn_context.model_context_window().is_some()
|
||||
&& step_context
|
||||
.settings
|
||||
.model_info
|
||||
.resolved_context_window()
|
||||
.is_some()
|
||||
{
|
||||
// Keep the legacy bridge hint when native Notes is disabled. A failed
|
||||
// native request must not fall back to the bridge.
|
||||
@@ -4291,7 +4299,7 @@ impl Session {
|
||||
};
|
||||
let (window_number, window_ids) = window;
|
||||
let context_items = self
|
||||
.build_initial_context_with_world_state(turn_context, world_state.as_ref())
|
||||
.build_initial_context_with_world_state(step_context, world_state.as_ref())
|
||||
.await
|
||||
.into_iter()
|
||||
.map(ResponseItemEnvelope::new)
|
||||
@@ -4350,7 +4358,7 @@ impl Session {
|
||||
// Full initial context resets the baseline; later turns persist only its changes.
|
||||
let (mut context_items, world_state_item) = if should_inject_full_context {
|
||||
let context_items = self
|
||||
.build_initial_context_with_world_state(turn_context, world_state.as_ref())
|
||||
.build_initial_context_with_world_state(step_context, world_state.as_ref())
|
||||
.await;
|
||||
let snapshot = world_state.snapshot();
|
||||
self.state
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
use crate::config::MultiAgentV2Config;
|
||||
use crate::context::MultiAgentRoleInstructions;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::session::step_context::StepContext;
|
||||
use codex_protocol::config_types::MultiAgentMode;
|
||||
use codex_protocol::openai_models::MultiAgentRoleMessages;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
@@ -65,15 +65,17 @@ pub(crate) struct ResolvedMultiAgentV2UsageHints {
|
||||
}
|
||||
|
||||
pub(super) fn usage_hint_text(
|
||||
turn_context: &TurnContext,
|
||||
step_context: &StepContext,
|
||||
session_source: &SessionSource,
|
||||
) -> Option<MultiAgentRoleInstructions> {
|
||||
let turn_context = step_context.turn.as_ref();
|
||||
if turn_context.multi_agent_version != MultiAgentVersion::V2 {
|
||||
return None;
|
||||
}
|
||||
|
||||
let catalog = turn_context
|
||||
.model_info()
|
||||
let catalog = step_context
|
||||
.settings
|
||||
.model_info
|
||||
.model_messages
|
||||
.as_ref()
|
||||
.and_then(|messages| messages.multi_agent.as_ref())
|
||||
@@ -152,13 +154,15 @@ pub(crate) fn resolve_usage_hints(
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn effective_multi_agent_mode(turn_context: &TurnContext) -> Option<MultiAgentMode> {
|
||||
pub(crate) fn effective_multi_agent_mode(step_context: &StepContext) -> Option<MultiAgentMode> {
|
||||
let turn_context = step_context.turn.as_ref();
|
||||
let settings = &step_context.settings;
|
||||
if turn_context.multi_agent_version != MultiAgentVersion::V2 {
|
||||
return None;
|
||||
}
|
||||
|
||||
let catalog_mode = turn_context
|
||||
.model_info()
|
||||
let catalog_mode = settings
|
||||
.model_info
|
||||
.model_messages
|
||||
.as_ref()
|
||||
.and_then(|messages| messages.multi_agent.as_ref())
|
||||
@@ -174,7 +178,7 @@ pub(crate) fn effective_multi_agent_mode(turn_context: &TurnContext) -> Option<M
|
||||
// of an effort-derived built-in policy.
|
||||
let multi_agent_mode = match mode_hint_text {
|
||||
Some(hint_text) => MultiAgentMode::Custom(hint_text.to_string()),
|
||||
None => match turn_context.effective_reasoning_effort() {
|
||||
None => match settings.effective_reasoning_effort() {
|
||||
Some(ReasoningEffort::Ultra) => catalog_mode
|
||||
.and_then(|messages| messages.proactive.clone())
|
||||
.map(MultiAgentMode::Custom)
|
||||
|
||||
@@ -90,6 +90,12 @@ impl ResolvedStepSettings {
|
||||
.cloned()
|
||||
}
|
||||
|
||||
pub(crate) fn effective_collaboration_mode(&self) -> CollaborationMode {
|
||||
let mut collaboration_mode = self.selected.collaboration_mode.clone();
|
||||
collaboration_mode.settings.model = self.model_info.slug.clone();
|
||||
collaboration_mode
|
||||
}
|
||||
|
||||
pub(crate) fn approval_policy(&self) -> AskForApproval {
|
||||
self.selected.approval_policy.value()
|
||||
}
|
||||
|
||||
@@ -74,6 +74,7 @@ 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;
|
||||
@@ -6416,9 +6417,13 @@ async fn build_initial_context(
|
||||
session: &Session,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
) -> Vec<ResponseItem> {
|
||||
let world_state = build_world_state_from_turn_context(session, turn_context).await;
|
||||
let step_context = StepContext::for_test(Arc::clone(turn_context));
|
||||
let world_state = session
|
||||
.build_world_state_for_step(&step_context)
|
||||
.await
|
||||
.expect("world state should build");
|
||||
session
|
||||
.build_initial_context_with_world_state(turn_context.as_ref(), &world_state)
|
||||
.build_initial_context_with_world_state(&step_context, &world_state)
|
||||
.await
|
||||
}
|
||||
|
||||
@@ -10123,8 +10128,9 @@ async fn build_initial_context_reuses_in_flight_recommendation_prewarm() {
|
||||
// Keep the OnceCell initializer unpolled while first-thread context joins its in-flight fetch.
|
||||
// This does not depend on how quickly the HTTP server returns its response.
|
||||
let world_state = WorldState::default();
|
||||
let step_context = StepContext::for_test(Arc::clone(&turn_context));
|
||||
let initial_context =
|
||||
session.build_initial_context_with_world_state(&turn_context, &world_state);
|
||||
session.build_initial_context_with_world_state(&step_context, &world_state);
|
||||
tokio::pin!(initial_context);
|
||||
assert!(futures::poll!(initial_context.as_mut()).is_pending());
|
||||
|
||||
@@ -10802,6 +10808,103 @@ async fn record_context_updates_and_set_reference_context_item_persists_split_fi
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn build_initial_context_uses_retained_step_after_model_change() {
|
||||
let (session, mut turn_context, _rx_event) =
|
||||
make_session_and_context_with_auth_and_config_and_rx(
|
||||
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;
|
||||
update_turn_settings_for_test(Arc::get_mut(&mut turn_context).unwrap(), |settings| {
|
||||
let model_info = Arc::make_mut(&mut settings.model_info);
|
||||
model_info.slug = "model-a".to_string();
|
||||
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()),
|
||||
});
|
||||
});
|
||||
session
|
||||
.set_previous_turn_settings(Some(PreviousTurnSettings {
|
||||
model: "base-model".to_string(),
|
||||
comp_hash: None,
|
||||
realtime_active: None,
|
||||
}))
|
||||
.await;
|
||||
let step_a = session
|
||||
.capture_step_context(Arc::clone(&turn_context), &CancellationToken::new())
|
||||
.await
|
||||
.unwrap();
|
||||
let world_a = Arc::new(session.build_world_state_for_step(&step_a).await.unwrap());
|
||||
let retained = crate::compact::InitialContextInjection::BeforeLastUserMessage {
|
||||
world_state: Arc::clone(&world_a),
|
||||
step_context: Arc::clone(&step_a),
|
||||
};
|
||||
let (initial_a, _) =
|
||||
crate::compact::build_compaction_initial_context(&session, &retained).await;
|
||||
|
||||
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);
|
||||
model_b
|
||||
.model_messages
|
||||
.as_mut()
|
||||
.unwrap()
|
||||
.instructions_template = Some("B instructions: {{ personality }}".to_string());
|
||||
turn_context
|
||||
.current_settings
|
||||
.store(Arc::new(ResolvedStepSettings::new(
|
||||
Arc::new(selected_b),
|
||||
Arc::new(model_b),
|
||||
/*fast_mode_enabled*/ false,
|
||||
)));
|
||||
let step_b = session
|
||||
.capture_step_context(Arc::clone(&turn_context), &CancellationToken::new())
|
||||
.await
|
||||
.unwrap();
|
||||
let world_b = session.build_world_state_for_step(&step_b).await.unwrap();
|
||||
let initial_b = session
|
||||
.build_initial_context_with_world_state(&step_b, &world_b)
|
||||
.await;
|
||||
let (restored_a, restored_world) =
|
||||
crate::compact::build_compaction_initial_context(&session, &retained).await;
|
||||
|
||||
assert_eq!(restored_a, initial_a);
|
||||
assert!(Arc::ptr_eq(restored_world.as_ref().unwrap(), &world_a));
|
||||
let initial_a = initial_a
|
||||
.into_iter()
|
||||
.map(ResponseItemEnvelope::into_item)
|
||||
.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("<context_window>"));
|
||||
assert!(b_text.contains("B instructions: pragmatic"));
|
||||
assert!(!b_text.contains("A instructions:"));
|
||||
assert!(
|
||||
b_text.contains("<context_window>"),
|
||||
"full-context metadata must use B's window even though the turn started without one"
|
||||
);
|
||||
assert_eq!(
|
||||
step_b.environments.to_selections(),
|
||||
step_a.environments.to_selections()
|
||||
);
|
||||
assert!(Arc::ptr_eq(&step_b.turn.config, &step_a.turn.config));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn build_initial_context_prepends_model_switch_message() {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
|
||||
@@ -369,16 +369,6 @@ impl TurnContext {
|
||||
self.initial_settings.personality()
|
||||
}
|
||||
|
||||
/// Legacy: returns the frozen initial-turn collaboration-mode developer instructions.
|
||||
/// Step-scoped consumers should use their captured `StepContext::settings`.
|
||||
pub(crate) fn collaboration_mode_developer_instructions(&self) -> &Option<String> {
|
||||
&self
|
||||
.initial_settings
|
||||
.selected_collaboration_mode()
|
||||
.settings
|
||||
.developer_instructions
|
||||
}
|
||||
|
||||
pub(crate) fn skills_snapshot(&self) -> Arc<HostSkillsSnapshot> {
|
||||
let Some(snapshot) = self.extension_data.get::<HostSkillsSnapshot>() else {
|
||||
unreachable!("every turn has a host skills snapshot");
|
||||
@@ -389,14 +379,7 @@ impl TurnContext {
|
||||
/// Legacy: returns the frozen initial-turn collaboration mode with the resolved model slug.
|
||||
/// Step-scoped consumers should use their captured `StepContext::settings`.
|
||||
pub(crate) fn collaboration_mode(&self) -> CollaborationMode {
|
||||
CollaborationMode {
|
||||
mode: self.mode(),
|
||||
settings: Settings {
|
||||
model: self.model_info().slug.clone(),
|
||||
reasoning_effort: self.reasoning_effort().cloned(),
|
||||
developer_instructions: self.collaboration_mode_developer_instructions().clone(),
|
||||
},
|
||||
}
|
||||
self.initial_settings.effective_collaboration_mode()
|
||||
}
|
||||
|
||||
pub(crate) fn plugin_attribution_for_command(
|
||||
|
||||
@@ -37,13 +37,14 @@ impl Session {
|
||||
step_context: &StepContext,
|
||||
) -> CodexResult<WorldState> {
|
||||
let turn_context = step_context.turn.as_ref();
|
||||
let settings = &step_context.settings;
|
||||
let model_info = settings.model_info.as_ref();
|
||||
let personality = settings.personality();
|
||||
tracing::trace!(
|
||||
selected_capability_root_count = step_context.selected_capability_roots.len(),
|
||||
"building step world state"
|
||||
);
|
||||
let model_instructions = turn_context
|
||||
.model_info()
|
||||
.get_model_instructions(turn_context.personality());
|
||||
let model_instructions = model_info.get_model_instructions(personality);
|
||||
let model_instructions = if !turn_context.config.update_plan_enabled
|
||||
&& turn_context.config.model_catalog.is_none()
|
||||
&& (turn_context.config.base_instructions.is_none()
|
||||
@@ -77,8 +78,8 @@ impl Session {
|
||||
base_instructions,
|
||||
)
|
||||
};
|
||||
let personality_is_baked = turn_context.model_info().supports_personality()
|
||||
&& base_instructions == model_instructions;
|
||||
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
|
||||
@@ -92,22 +93,21 @@ impl Session {
|
||||
};
|
||||
let mut world_state = WorldState::default();
|
||||
world_state.add_section(ModelInstructionsState::new(
|
||||
&turn_context.model_info().slug,
|
||||
&model_info.slug,
|
||||
previous_model.as_deref(),
|
||||
model_instructions,
|
||||
));
|
||||
if self.features.enabled(Feature::Personality) {
|
||||
let personality_instructions = turn_context.personality().and_then(|personality| {
|
||||
turn_context
|
||||
.model_info()
|
||||
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(
|
||||
&turn_context.model_info().slug,
|
||||
turn_context.personality(),
|
||||
&model_info.slug,
|
||||
personality,
|
||||
previous_context
|
||||
.as_ref()
|
||||
.map(|previous| previous.model.as_str())
|
||||
@@ -172,12 +172,12 @@ impl Session {
|
||||
let cwd = environment
|
||||
.and_then(|environment| environment.cwd().to_abs_path().ok())
|
||||
.unwrap_or_else(|| turn_context.cwd.clone());
|
||||
let model_messages = turn_context.model_info().model_messages.as_ref();
|
||||
let model_messages = model_info.model_messages.as_ref();
|
||||
world_state.add_section(PermissionsState::new(
|
||||
&permission_profile,
|
||||
step_context.settings.approval_policy(),
|
||||
settings.approval_policy(),
|
||||
ApprovalPromptContext::new(
|
||||
step_context.settings.approvals_reviewer(),
|
||||
settings.approvals_reviewer(),
|
||||
model_messages.and_then(|messages| messages.approvals.as_ref()),
|
||||
model_messages.and_then(|messages| messages.permissions.as_ref()),
|
||||
),
|
||||
@@ -197,9 +197,8 @@ impl Session {
|
||||
}
|
||||
if turn_context.config.include_collaboration_mode_instructions {
|
||||
world_state.add_section(CollaborationModeState::from_collaboration_mode(
|
||||
&turn_context.collaboration_mode(),
|
||||
turn_context
|
||||
.model_info()
|
||||
&settings.effective_collaboration_mode(),
|
||||
model_info
|
||||
.model_messages
|
||||
.as_ref()
|
||||
.and_then(|messages| messages.collaboration_modes.as_ref()),
|
||||
@@ -266,12 +265,12 @@ impl Session {
|
||||
false
|
||||
};
|
||||
let apps_usage_instructions_available =
|
||||
apps_available && turn_context.model_info().include_apps_usage_instructions;
|
||||
apps_available && model_info.include_apps_usage_instructions;
|
||||
world_state.add_section(AppsInstructionsState::new(
|
||||
apps_usage_instructions_available,
|
||||
));
|
||||
let plugins_usage_instructions_available = step_context.mcp.plugins_available()
|
||||
&& turn_context.model_info().include_plugin_usage_instructions;
|
||||
let plugins_usage_instructions_available =
|
||||
step_context.mcp.plugins_available() && model_info.include_plugin_usage_instructions;
|
||||
world_state.add_section(PluginsInstructionsState::new(
|
||||
plugins_usage_instructions_available,
|
||||
));
|
||||
@@ -314,10 +313,10 @@ impl Session {
|
||||
}
|
||||
}
|
||||
let mut multi_agent_mode = MultiAgentModeState::new(
|
||||
super::multi_agents::effective_multi_agent_mode(turn_context),
|
||||
super::multi_agents::effective_multi_agent_mode(step_context),
|
||||
);
|
||||
if let Some(usage_hint_text) =
|
||||
super::multi_agents::usage_hint_text(turn_context, &turn_context.session_source)
|
||||
super::multi_agents::usage_hint_text(step_context, &turn_context.session_source)
|
||||
{
|
||||
let usage_hint = MultiAgentUsageHintState::new(usage_hint_text);
|
||||
multi_agent_mode = multi_agent_mode.with_usage_hint(&usage_hint);
|
||||
|
||||
Reference in new issue
Block a user