mirror of
https://github.com/openai/codex.git
synced 2026-10-22 17:58:46 +00:00
Record base instructions in incremental tool history (#51188)
## What changed - Record nonempty base instructions as a developer message after tool declarations at the start of each incremental tool context window, and omit the request-level `instructions` field. - Preserve this prefix when rebuilding history after local or remote compaction, while keeping the compaction summary last. - Count recorded base instructions once when estimating context usage and trimming tool outputs. ## Testing Add regression coverage for instruction placement, omission of request-level instructions, compaction ordering, and token accounting. Exercise remote compaction trimming with both request-level and recorded base instructions. GitOrigin-RevId: 6a6fda92c5c6eb33852a77b38d34c028cf78b164
This commit is contained in:
@@ -33,6 +33,7 @@ use crate::context::RecommendedPluginsInstructions;
|
||||
use crate::context::world_state::Placement;
|
||||
use crate::context::world_state::WorldState;
|
||||
use crate::context::world_state::WorldStateSnapshot;
|
||||
use crate::context::world_state::WorldStateUpdate;
|
||||
use crate::context::world_state::WorldStateUpdateContent;
|
||||
use crate::context::world_state::split_prefix_updates;
|
||||
use crate::current_time::TimeProvider;
|
||||
@@ -4182,13 +4183,12 @@ impl Session {
|
||||
}
|
||||
|
||||
/// `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.
|
||||
/// Returns rendered items with placement preserved for the caller's history layout.
|
||||
pub(crate) async fn build_initial_context_with_world_state(
|
||||
&self,
|
||||
step_context: &StepContext,
|
||||
world_state: &WorldState,
|
||||
) -> (Vec<ResponseItem>, WorldStateSnapshot) {
|
||||
) -> (Vec<WorldStateUpdate>, WorldStateSnapshot) {
|
||||
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);
|
||||
@@ -4331,8 +4331,9 @@ impl Session {
|
||||
.render_fragment(),
|
||||
);
|
||||
}
|
||||
let (world_state_snapshot, updates) = world_state.render_full();
|
||||
let (mut items, context) = split_prefix_updates(updates);
|
||||
let (snapshot, updates) = world_state.render_full();
|
||||
let (mut prefix, context) = split_prefix_updates(updates);
|
||||
let mut items = Vec::with_capacity(4);
|
||||
let mut context = context.into_iter();
|
||||
// Keep the initial-context grouping within each fragment run, but never across an item.
|
||||
loop {
|
||||
@@ -4445,10 +4446,18 @@ impl Session {
|
||||
}
|
||||
}
|
||||
// New context windows and compaction install these items directly into replacement history.
|
||||
for item in &mut items {
|
||||
for item in prefix.iter_mut().chain(&mut items) {
|
||||
item.set_turn_id_if_missing(&turn_context.sub_id);
|
||||
}
|
||||
(items, world_state_snapshot)
|
||||
let updates = prefix
|
||||
.into_iter()
|
||||
.map(WorldStateUpdate::prefix_item)
|
||||
.chain(items.into_iter().map(|item| WorldStateUpdate {
|
||||
placement: Placement::Standalone,
|
||||
content: WorldStateUpdateContent::Item(Box::new(item)),
|
||||
}))
|
||||
.collect();
|
||||
(updates, snapshot)
|
||||
}
|
||||
|
||||
#[tracing::instrument(level = "trace", skip_all, fields(item_count = items.len()))]
|
||||
@@ -4543,14 +4552,15 @@ impl Session {
|
||||
state.start_new_context_window()
|
||||
};
|
||||
let (window_number, window_ids) = window;
|
||||
let (context_items, world_state_snapshot) = self
|
||||
let (context_updates, world_state_snapshot) = self
|
||||
.build_initial_context_with_world_state(step_context, world_state.as_ref())
|
||||
.await;
|
||||
let context_items = context_items
|
||||
.into_iter()
|
||||
.map(ResponseItemEnvelope::new)
|
||||
.chain(retained_client_developer_messages)
|
||||
.collect();
|
||||
let context_items =
|
||||
crate::context_manager::updates::merge_world_state_updates(context_updates)
|
||||
.into_iter()
|
||||
.map(ResponseItemEnvelope::new)
|
||||
.chain(retained_client_developer_messages)
|
||||
.collect();
|
||||
let turn_context_item = step_context.to_turn_context_item();
|
||||
self.replace_compacted_history(
|
||||
context_items,
|
||||
@@ -4605,9 +4615,11 @@ impl Session {
|
||||
let world_state = Arc::new(self.build_world_state_for_step(step_context).await?);
|
||||
// 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, snapshot) = self
|
||||
let (context_updates, snapshot) = self
|
||||
.build_initial_context_with_world_state(step_context, world_state.as_ref())
|
||||
.await;
|
||||
let context_items =
|
||||
crate::context_manager::updates::merge_world_state_updates(context_updates);
|
||||
self.state
|
||||
.lock()
|
||||
.await
|
||||
|
||||
@@ -6654,10 +6654,10 @@ async fn build_initial_context(
|
||||
.build_world_state_for_step(&step_context)
|
||||
.await
|
||||
.expect("world state should build");
|
||||
session
|
||||
let (updates, _) = session
|
||||
.build_initial_context_with_world_state(&step_context, &world_state)
|
||||
.await
|
||||
.0
|
||||
.await;
|
||||
crate::context_manager::updates::merge_world_state_updates(updates)
|
||||
}
|
||||
|
||||
pub(crate) async fn build_world_state_from_turn_context(
|
||||
@@ -10568,6 +10568,8 @@ async fn build_initial_context_reuses_in_flight_recommendation_prewarm() {
|
||||
assert!(futures::poll!(initial_context.as_mut()).is_pending());
|
||||
|
||||
let (_, (initial_context, _)) = tokio::join!(prewarm, initial_context);
|
||||
let initial_context =
|
||||
crate::context_manager::updates::merge_world_state_updates(initial_context);
|
||||
assert_eq!(
|
||||
developer_input_texts(&initial_context)
|
||||
.into_iter()
|
||||
@@ -11298,8 +11300,13 @@ async fn build_initial_context_uses_retained_step_after_model_change() {
|
||||
.await
|
||||
.unwrap();
|
||||
let world_a = Arc::new(session.build_world_state_for_step(&step_a).await.unwrap());
|
||||
let (initial_a, _) =
|
||||
crate::compact::build_compaction_initial_context(&session, &step_a, &world_a).await;
|
||||
let (initial_a, _) = crate::compact::build_compaction_replacement_history(
|
||||
&session,
|
||||
&step_a,
|
||||
&world_a,
|
||||
Vec::new(),
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut selected_b = step_a.settings.selected().clone();
|
||||
selected_b.collaboration_mode.settings.model = "model-b".to_string();
|
||||
@@ -11328,15 +11335,21 @@ async fn build_initial_context_uses_retained_step_after_model_change() {
|
||||
.build_initial_context_with_world_state(&step_b, &world_b)
|
||||
.await
|
||||
.0;
|
||||
let initial_b = crate::context_manager::updates::merge_world_state_updates(initial_b);
|
||||
let turn_contributions_b = session.build_turn_context_contribution_items(&step_b).await;
|
||||
let (restored_a, restored_snapshot) =
|
||||
crate::compact::build_compaction_initial_context(&session, &step_a, &world_a).await;
|
||||
let (restored_a, restored_snapshot) = crate::compact::build_compaction_replacement_history(
|
||||
&session,
|
||||
&step_a,
|
||||
&world_a,
|
||||
Vec::new(),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(restored_a, initial_a);
|
||||
assert_eq!(restored_snapshot, world_a.render_full().0);
|
||||
let initial_a = initial_a
|
||||
.into_iter()
|
||||
.map(ResponseItemEnvelope::into_item)
|
||||
.map(|item| item.item)
|
||||
.collect::<Vec<_>>();
|
||||
let a_text = developer_input_texts(&initial_a).join("\n");
|
||||
let b_text = developer_input_texts(&initial_b).join("\n");
|
||||
|
||||
@@ -1117,20 +1117,23 @@ async fn compaction_initial_context_preserves_separate_guardian_developer_messag
|
||||
.await
|
||||
.expect("world state should build"),
|
||||
);
|
||||
let (refreshed, _) =
|
||||
crate::compact::build_compaction_initial_context(&session, &step_context, &world_state)
|
||||
.await;
|
||||
let (refreshed, _) = crate::compact::build_compaction_replacement_history(
|
||||
&session,
|
||||
&step_context,
|
||||
&world_state,
|
||||
Vec::new(),
|
||||
)
|
||||
.await;
|
||||
|
||||
let developer_messages = refreshed
|
||||
.iter()
|
||||
.filter_map(|envelope| match &envelope.item {
|
||||
.map(|item| &item.item)
|
||||
.filter_map(|item| match item {
|
||||
ResponseItem::Message { role, content, .. } if role == "developer" => {
|
||||
crate::content_items_to_text(content).map(|text| {
|
||||
(
|
||||
text,
|
||||
envelope
|
||||
.item
|
||||
.executed_tool_call_metadata()
|
||||
item.executed_tool_call_metadata()
|
||||
.and_then(|metadata| metadata.content_item_kinds.clone()),
|
||||
)
|
||||
})
|
||||
|
||||
@@ -1583,7 +1583,14 @@ pub(crate) fn build_prompt(
|
||||
step_context.tool_router.model_visible_specs()
|
||||
},
|
||||
parallel_tool_calls: true,
|
||||
base_instructions,
|
||||
base_instructions: if step_context.uses_incremental_tools() {
|
||||
BaseInstructions {
|
||||
text: String::new(),
|
||||
provenance: None,
|
||||
}
|
||||
} else {
|
||||
base_instructions
|
||||
},
|
||||
output_schema: turn_context.final_output_json_schema.clone(),
|
||||
output_schema_strict: !crate::guardian::is_basic_session_source(
|
||||
&turn_context.session_source,
|
||||
|
||||
@@ -6,6 +6,7 @@ use crate::connectors;
|
||||
use crate::context::TokenBudgetContext;
|
||||
use crate::context::world_state::AgentsMdState;
|
||||
use crate::context::world_state::AppsInstructionsState;
|
||||
use crate::context::world_state::BaseInstructionsState;
|
||||
use crate::context::world_state::CollaborationModeState;
|
||||
use crate::context::world_state::CompactPermissionsState;
|
||||
use crate::context::world_state::ContextWindowGuidanceState;
|
||||
@@ -122,6 +123,7 @@ impl Session {
|
||||
let specs = step_context.tool_router.model_visible_specs();
|
||||
let definitions = codex_tools::create_tools_json_for_responses_lite(&specs)?;
|
||||
world_state.add_section(TopLevelToolsState::new(definitions)?);
|
||||
world_state.add_section(BaseInstructionsState(base_instructions));
|
||||
}
|
||||
world_state.add_section(ModelInstructionsState::new(
|
||||
&model_info.slug,
|
||||
|
||||
@@ -13,6 +13,8 @@ use crate::context::world_state::WorldState;
|
||||
use crate::context::world_state::WorldStateSection;
|
||||
use crate::context::world_state::WorldStateUpdate;
|
||||
use crate::context::world_state::WorldStateUpdateContent;
|
||||
use crate::context::world_state::split_prefix_updates;
|
||||
use crate::context_manager::updates::merge_world_state_updates;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use core_test_support::responses::strip_metadata_from_items;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -85,14 +87,20 @@ async fn initial_context_preserves_world_state_items_and_snapshot() {
|
||||
let step_context = StepContext::for_test(Arc::new(turn_context));
|
||||
let mut world_state = WorldState::default();
|
||||
world_state.add_section(ItemSection);
|
||||
let (items, snapshot) = session
|
||||
let (updates, snapshot) = session
|
||||
.build_initial_context_with_world_state(&step_context, &world_state)
|
||||
.await;
|
||||
let (prefix, context) = split_prefix_updates(updates);
|
||||
assert_eq!(
|
||||
strip_metadata_from_items(&items),
|
||||
strip_metadata_from_items(&prefix),
|
||||
strip_metadata_from_items(&[
|
||||
additional_tools("example"),
|
||||
ContextualUserFragment::into(BaseInstructionsFragment("base instructions".to_string())),
|
||||
]),
|
||||
);
|
||||
assert_eq!(
|
||||
strip_metadata_from_items(&merge_world_state_updates(context)),
|
||||
strip_metadata_from_items(&[
|
||||
ContextualUserFragment::into(DeveloperInstructions::new("before item")),
|
||||
ContextualUserFragment::into(DeveloperInstructions::new("separate before item")),
|
||||
ContextualUserFragment::into(UserInstructions {
|
||||
|
||||
Reference in New Issue
Block a user