mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Add collaboration developer instructions (#9424)
- Add additional instructions when they are available - Make sure to update them on change either UserInput or UserTurn
This commit is contained in:
+123
-35
@@ -815,7 +815,7 @@ impl Session {
|
||||
match conversation_history {
|
||||
InitialHistory::New => {
|
||||
// Build and record initial items (user instructions + environment context)
|
||||
let items = self.build_initial_context(&turn_context);
|
||||
let items = self.build_initial_context(&turn_context).await;
|
||||
self.record_conversation_items(&turn_context, &items).await;
|
||||
// Ensure initial items are visible to immediate readers (e.g., tests, forks).
|
||||
self.flush_rollout().await;
|
||||
@@ -853,8 +853,9 @@ impl Session {
|
||||
}
|
||||
|
||||
// Always add response items to conversation history
|
||||
let reconstructed_history =
|
||||
self.reconstruct_history_from_rollout(&turn_context, &rollout_items);
|
||||
let reconstructed_history = self
|
||||
.reconstruct_history_from_rollout(&turn_context, &rollout_items)
|
||||
.await;
|
||||
if !reconstructed_history.is_empty() {
|
||||
self.record_into_history(&reconstructed_history, &turn_context)
|
||||
.await;
|
||||
@@ -873,7 +874,7 @@ impl Session {
|
||||
}
|
||||
|
||||
// Append the current session's initial context after the reconstructed history.
|
||||
let initial_context = self.build_initial_context(&turn_context);
|
||||
let initial_context = self.build_initial_context(&turn_context).await;
|
||||
self.record_conversation_items(&turn_context, &initial_context)
|
||||
.await;
|
||||
// Flush after seeding history and any persisted rollout copy.
|
||||
@@ -1061,6 +1062,50 @@ impl Session {
|
||||
)
|
||||
}
|
||||
|
||||
fn build_collaboration_mode_update_item(
|
||||
&self,
|
||||
previous_collaboration_mode: &CollaborationMode,
|
||||
next_collaboration_mode: Option<&CollaborationMode>,
|
||||
) -> Option<ResponseItem> {
|
||||
if let Some(next_mode) = next_collaboration_mode {
|
||||
if previous_collaboration_mode == next_mode {
|
||||
return None;
|
||||
}
|
||||
// If the next mode has empty developer instructions, this returns None and we emit no
|
||||
// update, so prior collaboration instructions remain in the prompt history.
|
||||
Some(DeveloperInstructions::from_collaboration_mode(next_mode)?.into())
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
fn build_settings_update_items(
|
||||
&self,
|
||||
previous_context: Option<&Arc<TurnContext>>,
|
||||
current_context: &TurnContext,
|
||||
previous_collaboration_mode: &CollaborationMode,
|
||||
next_collaboration_mode: Option<&CollaborationMode>,
|
||||
) -> Vec<ResponseItem> {
|
||||
let mut update_items = Vec::new();
|
||||
if let Some(env_item) =
|
||||
self.build_environment_update_item(previous_context, current_context)
|
||||
{
|
||||
update_items.push(env_item);
|
||||
}
|
||||
if let Some(permissions_item) =
|
||||
self.build_permissions_update_item(previous_context, current_context)
|
||||
{
|
||||
update_items.push(permissions_item);
|
||||
}
|
||||
if let Some(collaboration_mode_item) = self.build_collaboration_mode_update_item(
|
||||
previous_collaboration_mode,
|
||||
next_collaboration_mode,
|
||||
) {
|
||||
update_items.push(collaboration_mode_item);
|
||||
}
|
||||
update_items
|
||||
}
|
||||
|
||||
/// Persist the event to rollout and send it to clients.
|
||||
pub(crate) async fn send_event(&self, turn_context: &TurnContext, msg: EventMsg) {
|
||||
let legacy_source = msg.clone();
|
||||
@@ -1299,7 +1344,7 @@ impl Session {
|
||||
self.send_raw_response_items(turn_context, items).await;
|
||||
}
|
||||
|
||||
fn reconstruct_history_from_rollout(
|
||||
async fn reconstruct_history_from_rollout(
|
||||
&self,
|
||||
turn_context: &TurnContext,
|
||||
rollout_items: &[RolloutItem],
|
||||
@@ -1319,7 +1364,7 @@ impl Session {
|
||||
} else {
|
||||
let user_messages = collect_user_messages(history.raw_items());
|
||||
let rebuilt = compact::build_compacted_history(
|
||||
self.build_initial_context(turn_context),
|
||||
self.build_initial_context(turn_context).await,
|
||||
&user_messages,
|
||||
&compacted.message,
|
||||
);
|
||||
@@ -1389,7 +1434,10 @@ impl Session {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn build_initial_context(&self, turn_context: &TurnContext) -> Vec<ResponseItem> {
|
||||
pub(crate) async fn build_initial_context(
|
||||
&self,
|
||||
turn_context: &TurnContext,
|
||||
) -> Vec<ResponseItem> {
|
||||
let mut items = Vec::<ResponseItem>::with_capacity(4);
|
||||
let shell = self.user_shell();
|
||||
items.push(
|
||||
@@ -1403,6 +1451,16 @@ impl Session {
|
||||
if let Some(developer_instructions) = turn_context.developer_instructions.as_deref() {
|
||||
items.push(DeveloperInstructions::new(developer_instructions.to_string()).into());
|
||||
}
|
||||
// Add developer instructions from collaboration_mode if they exist and are non-empty
|
||||
let collaboration_mode = {
|
||||
let state = self.state.lock().await;
|
||||
state.session_configuration.collaboration_mode.clone()
|
||||
};
|
||||
if let Some(collab_instructions) =
|
||||
DeveloperInstructions::from_collaboration_mode(&collaboration_mode)
|
||||
{
|
||||
items.push(collab_instructions.into());
|
||||
}
|
||||
if let Some(user_instructions) = turn_context.user_instructions.as_deref() {
|
||||
items.push(
|
||||
UserInstructions {
|
||||
@@ -1984,6 +2042,18 @@ mod handlers {
|
||||
sub_id: String,
|
||||
updates: SessionSettingsUpdate,
|
||||
) {
|
||||
let previous_context = sess
|
||||
.new_default_turn_with_sub_id(sess.next_internal_sub_id())
|
||||
.await;
|
||||
let previous_collaboration_mode = sess
|
||||
.state
|
||||
.lock()
|
||||
.await
|
||||
.session_configuration
|
||||
.collaboration_mode
|
||||
.clone();
|
||||
let next_collaboration_mode = updates.collaboration_mode.clone();
|
||||
|
||||
if let Err(err) = sess.update_settings(updates).await {
|
||||
sess.send_event_raw(Event {
|
||||
id: sub_id,
|
||||
@@ -1993,6 +2063,19 @@ mod handlers {
|
||||
}),
|
||||
})
|
||||
.await;
|
||||
return;
|
||||
}
|
||||
|
||||
let current_context = sess.new_default_turn_with_sub_id(sub_id).await;
|
||||
let update_items = sess.build_settings_update_items(
|
||||
Some(&previous_context),
|
||||
¤t_context,
|
||||
&previous_collaboration_mode,
|
||||
next_collaboration_mode.as_ref(),
|
||||
);
|
||||
if !update_items.is_empty() {
|
||||
sess.record_conversation_items(¤t_context, &update_items)
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2046,6 +2129,14 @@ mod handlers {
|
||||
_ => unreachable!(),
|
||||
};
|
||||
|
||||
let previous_collaboration_mode = sess
|
||||
.state
|
||||
.lock()
|
||||
.await
|
||||
.session_configuration
|
||||
.collaboration_mode
|
||||
.clone();
|
||||
let next_collaboration_mode = updates.collaboration_mode.clone();
|
||||
let Ok(current_context) = sess.new_turn_with_sub_id(sub_id, updates).await else {
|
||||
// new_turn_with_sub_id already emits the error event.
|
||||
return;
|
||||
@@ -2057,17 +2148,12 @@ mod handlers {
|
||||
|
||||
// Attempt to inject input into current task
|
||||
if let Err(items) = sess.inject_input(items).await {
|
||||
let mut update_items = Vec::new();
|
||||
if let Some(env_item) =
|
||||
sess.build_environment_update_item(previous_context.as_ref(), ¤t_context)
|
||||
{
|
||||
update_items.push(env_item);
|
||||
}
|
||||
if let Some(permissions_item) =
|
||||
sess.build_permissions_update_item(previous_context.as_ref(), ¤t_context)
|
||||
{
|
||||
update_items.push(permissions_item);
|
||||
}
|
||||
let update_items = sess.build_settings_update_items(
|
||||
previous_context.as_ref(),
|
||||
¤t_context,
|
||||
&previous_collaboration_mode,
|
||||
next_collaboration_mode.as_ref(),
|
||||
);
|
||||
if !update_items.is_empty() {
|
||||
sess.record_conversation_items(¤t_context, &update_items)
|
||||
.await;
|
||||
@@ -3178,9 +3264,11 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn reconstruct_history_matches_live_compactions() {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
let (rollout_items, expected) = sample_rollout(&session, &turn_context);
|
||||
let (rollout_items, expected) = sample_rollout(&session, &turn_context).await;
|
||||
|
||||
let reconstructed = session.reconstruct_history_from_rollout(&turn_context, &rollout_items);
|
||||
let reconstructed = session
|
||||
.reconstruct_history_from_rollout(&turn_context, &rollout_items)
|
||||
.await;
|
||||
|
||||
assert_eq!(expected, reconstructed);
|
||||
}
|
||||
@@ -3188,7 +3276,7 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn record_initial_history_reconstructs_resumed_transcript() {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
let (rollout_items, mut expected) = sample_rollout(&session, &turn_context);
|
||||
let (rollout_items, mut expected) = sample_rollout(&session, &turn_context).await;
|
||||
|
||||
session
|
||||
.record_initial_history(InitialHistory::Resumed(ResumedHistory {
|
||||
@@ -3198,7 +3286,7 @@ mod tests {
|
||||
}))
|
||||
.await;
|
||||
|
||||
expected.extend(session.build_initial_context(&turn_context));
|
||||
expected.extend(session.build_initial_context(&turn_context).await);
|
||||
let history = session.state.lock().await.clone_history();
|
||||
assert_eq!(expected, history.raw_items());
|
||||
}
|
||||
@@ -3206,7 +3294,7 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn record_initial_history_seeds_token_info_from_rollout() {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
let (mut rollout_items, _expected) = sample_rollout(&session, &turn_context);
|
||||
let (mut rollout_items, _expected) = sample_rollout(&session, &turn_context).await;
|
||||
|
||||
let info1 = TokenUsageInfo {
|
||||
total_token_usage: TokenUsage {
|
||||
@@ -3283,13 +3371,13 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn record_initial_history_reconstructs_forked_transcript() {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
let (rollout_items, mut expected) = sample_rollout(&session, &turn_context);
|
||||
let (rollout_items, mut expected) = sample_rollout(&session, &turn_context).await;
|
||||
|
||||
session
|
||||
.record_initial_history(InitialHistory::Forked(rollout_items))
|
||||
.await;
|
||||
|
||||
expected.extend(session.build_initial_context(&turn_context));
|
||||
expected.extend(session.build_initial_context(&turn_context).await);
|
||||
let history = session.state.lock().await.clone_history();
|
||||
assert_eq!(expected, history.raw_items());
|
||||
}
|
||||
@@ -3298,7 +3386,7 @@ mod tests {
|
||||
async fn thread_rollback_drops_last_turn_from_history() {
|
||||
let (sess, tc, rx) = make_session_and_context_with_rx().await;
|
||||
|
||||
let initial_context = sess.build_initial_context(tc.as_ref());
|
||||
let initial_context = sess.build_initial_context(tc.as_ref()).await;
|
||||
sess.record_into_history(&initial_context, tc.as_ref())
|
||||
.await;
|
||||
|
||||
@@ -3355,7 +3443,7 @@ mod tests {
|
||||
async fn thread_rollback_clears_history_when_num_turns_exceeds_existing_turns() {
|
||||
let (sess, tc, rx) = make_session_and_context_with_rx().await;
|
||||
|
||||
let initial_context = sess.build_initial_context(tc.as_ref());
|
||||
let initial_context = sess.build_initial_context(tc.as_ref()).await;
|
||||
sess.record_into_history(&initial_context, tc.as_ref())
|
||||
.await;
|
||||
|
||||
@@ -3381,7 +3469,7 @@ mod tests {
|
||||
async fn thread_rollback_fails_when_turn_in_progress() {
|
||||
let (sess, tc, rx) = make_session_and_context_with_rx().await;
|
||||
|
||||
let initial_context = sess.build_initial_context(tc.as_ref());
|
||||
let initial_context = sess.build_initial_context(tc.as_ref()).await;
|
||||
sess.record_into_history(&initial_context, tc.as_ref())
|
||||
.await;
|
||||
|
||||
@@ -3402,7 +3490,7 @@ mod tests {
|
||||
async fn thread_rollback_fails_when_num_turns_is_zero() {
|
||||
let (sess, tc, rx) = make_session_and_context_with_rx().await;
|
||||
|
||||
let initial_context = sess.build_initial_context(tc.as_ref());
|
||||
let initial_context = sess.build_initial_context(tc.as_ref()).await;
|
||||
sess.record_into_history(&initial_context, tc.as_ref())
|
||||
.await;
|
||||
|
||||
@@ -4188,14 +4276,14 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
fn sample_rollout(
|
||||
async fn sample_rollout(
|
||||
session: &Session,
|
||||
turn_context: &TurnContext,
|
||||
) -> (Vec<RolloutItem>, Vec<ResponseItem>) {
|
||||
let mut rollout_items = Vec::new();
|
||||
let mut live_history = ContextManager::new();
|
||||
|
||||
let initial_context = session.build_initial_context(turn_context);
|
||||
let initial_context = session.build_initial_context(turn_context).await;
|
||||
for item in &initial_context {
|
||||
rollout_items.push(RolloutItem::ResponseItem(item.clone()));
|
||||
}
|
||||
@@ -4225,7 +4313,7 @@ mod tests {
|
||||
let snapshot1 = live_history.clone().for_prompt();
|
||||
let user_messages1 = collect_user_messages(&snapshot1);
|
||||
let rebuilt1 = compact::build_compacted_history(
|
||||
session.build_initial_context(turn_context),
|
||||
session.build_initial_context(turn_context).await,
|
||||
&user_messages1,
|
||||
summary1,
|
||||
);
|
||||
@@ -4259,7 +4347,7 @@ mod tests {
|
||||
let snapshot2 = live_history.clone().for_prompt();
|
||||
let user_messages2 = collect_user_messages(&snapshot2);
|
||||
let rebuilt2 = compact::build_compacted_history(
|
||||
session.build_initial_context(turn_context),
|
||||
session.build_initial_context(turn_context).await,
|
||||
&user_messages2,
|
||||
summary2,
|
||||
);
|
||||
@@ -4277,7 +4365,7 @@ mod tests {
|
||||
}],
|
||||
};
|
||||
live_history.record_items(std::iter::once(&user3), turn_context.truncation_policy);
|
||||
rollout_items.push(RolloutItem::ResponseItem(user3.clone()));
|
||||
rollout_items.push(RolloutItem::ResponseItem(user3));
|
||||
|
||||
let assistant3 = ResponseItem::Message {
|
||||
id: None,
|
||||
@@ -4287,7 +4375,7 @@ mod tests {
|
||||
}],
|
||||
};
|
||||
live_history.record_items(std::iter::once(&assistant3), turn_context.truncation_policy);
|
||||
rollout_items.push(RolloutItem::ResponseItem(assistant3.clone()));
|
||||
rollout_items.push(RolloutItem::ResponseItem(assistant3));
|
||||
|
||||
(rollout_items, live_history.for_prompt())
|
||||
}
|
||||
|
||||
@@ -167,7 +167,7 @@ async fn run_compact_task_inner(
|
||||
let summary_text = format!("{SUMMARY_PREFIX}\n{summary_suffix}");
|
||||
let user_messages = collect_user_messages(history_items);
|
||||
|
||||
let initial_context = sess.build_initial_context(turn_context.as_ref());
|
||||
let initial_context = sess.build_initial_context(turn_context.as_ref()).await;
|
||||
let mut new_history = build_compacted_history(initial_context, &user_messages, &summary_text);
|
||||
let ghost_snapshots: Vec<ResponseItem> = history_items
|
||||
.iter()
|
||||
|
||||
@@ -189,7 +189,7 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn ignores_session_prefix_messages_when_truncating_rollout_from_start() {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
let mut items = session.build_initial_context(&turn_context);
|
||||
let mut items = session.build_initial_context(&turn_context).await;
|
||||
items.push(user_msg("feature request"));
|
||||
items.push(assistant_msg("ack"));
|
||||
items.push(user_msg("second question"));
|
||||
|
||||
@@ -467,7 +467,7 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn ignores_session_prefix_messages_when_truncating() {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
let mut items = session.build_initial_context(&turn_context);
|
||||
let mut items = session.build_initial_context(&turn_context).await;
|
||||
items.push(user_msg("feature request"));
|
||||
items.push(assistant_msg("ack"));
|
||||
items.push(user_msg("second question"));
|
||||
|
||||
Reference in New Issue
Block a user