Support ordered response items in world-state context updates (#50441)

## Why

World-state sections could return only a single optional text fragment, with message separation controlled by the fragment itself. They could not contribute explicit `ResponseItem` values alongside instructions.

## What changed

- Let sections return ordered `WorldStateUpdate` values containing fragments or response items, with `Prefix`, `Standalone`, or `Mergeable` placement.
- Preserve explicit item boundaries when grouping context updates, and place prefix updates first when assembling initial context.
- Move standalone-message decisions from fragments to world-state updates and retain snapshot handling.

## Testing

Add tests for mixed fragment/item ordering, role and standalone boundaries, initial-context prefix placement, and snapshot restoration without repeated updates.

GitOrigin-RevId: c2514c3c98538da83f5140c10a9d89f531de6549
This commit is contained in:
pakrym-oai
2026-10-02 22:12:07 +00:00
committed by copyberry
parent 12a30d4e6d
commit 0df76892b1
53 changed files with 931 additions and 337 deletions
@@ -67,11 +67,6 @@ pub trait ContextualUserFragment {
/// Returns a stable `<feature>.<name>` classification, using `generic` for shared fragments.
fn content_kind(&self) -> ContentItemKind;
/// Whether this fragment must be recorded as its own response item.
fn requires_separate_message(&self) -> bool {
false
}
fn markers(&self) -> (&'static str, &'static str);
fn body(&self) -> String;
@@ -13,10 +13,6 @@ impl ContextualUserFragment for BaseInstructionsFragment {
"developer"
}
fn requires_separate_message(&self) -> bool {
true
}
fn markers(&self) -> (&'static str, &'static str) {
Self::type_markers()
}
@@ -24,10 +24,6 @@ impl ContextualUserFragment for GuardianPolicy {
"developer"
}
fn requires_separate_message(&self) -> bool {
true
}
fn markers(&self) -> (&'static str, &'static str) {
Self::type_markers()
}
@@ -41,10 +41,6 @@ impl ContextualUserFragment for ImageResizeNotice {
"developer"
}
fn requires_separate_message(&self) -> bool {
true
}
fn markers(&self) -> (&'static str, &'static str) {
Self::type_markers()
}
-3
View File
@@ -27,9 +27,6 @@ impl ContextualUserFragment for MemoryContextFragment {
.to_string(),
)
}
fn requires_separate_message(&self) -> bool {
true
}
fn markers(&self) -> (&'static str, &'static str) {
Self::type_markers()
}
@@ -23,10 +23,6 @@ impl ContextualUserFragment for ModelSwitchInstructions {
"developer"
}
fn requires_separate_message(&self) -> bool {
true
}
fn markers(&self) -> (&'static str, &'static str) {
Self::type_markers()
}
@@ -24,10 +24,6 @@ impl ContextualUserFragment for MultiAgentUsageHint {
"developer"
}
fn requires_separate_message(&self) -> bool {
true
}
fn markers(&self) -> (&'static str, &'static str) {
Self::type_markers()
}
@@ -2,6 +2,7 @@ use super::ContextualUserFragment;
use super::world_state::PreviousSectionState;
use super::world_state::WorldStateSection;
use crate::context::world_state::SectionTransition;
use crate::context::world_state::WorldStateUpdate;
use codex_protocol::AgentPath;
use codex_protocol::models::ContentItemKind;
use codex_protocol::protocol::CONTEXT_WINDOW_CLOSE_TAG;
@@ -46,10 +47,6 @@ impl ContextualUserFragment for TokenBudgetContext {
"developer"
}
fn requires_separate_message(&self) -> bool {
true
}
fn markers(&self) -> (&'static str, &'static str) {
Self::type_markers()
}
@@ -87,7 +84,14 @@ impl WorldStateSection for TokenBudgetContext {
let current = self.agent_path.clone();
let fragment = matches!(previous, PreviousSectionState::Known(agent_path) if agent_path != &self.agent_path)
.then(|| Box::new(self.clone()) as Box<dyn ContextualUserFragment>);
(Some(current), fragment)
(
Some(current),
fragment
.into_iter()
.map(WorldStateUpdate::boxed_fragment)
.map(WorldStateUpdate::standalone)
.collect(),
)
}
}
@@ -1,6 +1,7 @@
use super::PreviousSectionState;
use super::SectionTransition;
use super::WorldStateSection;
use super::WorldStateUpdate;
use crate::agents_md::LoadedAgentsMd;
use crate::context::ContextualUserFragment;
use crate::context::UserInstructions;
@@ -52,7 +53,7 @@ impl WorldStateSection for AgentsMdState {
None => AgentsMdSnapshot::default(),
};
if matches!(previous, PreviousSectionState::Known(previous) if previous == &current) {
return (None, None);
return (None, Vec::new());
}
let previous_may_contain_instructions = match previous {
@@ -70,9 +71,12 @@ impl WorldStateSection for AgentsMdState {
directory: None,
text: REMOVAL_NOTICE.to_string(),
},
(None, false) => return (Some(current), None),
(None, false) => return (Some(current), Vec::new()),
};
(Some(current), Some(Box::new(instructions)))
(
Some(current),
vec![WorldStateUpdate::fragment(instructions)],
)
}
}
@@ -1,6 +1,7 @@
use super::PreviousSectionState;
use super::SectionTransition;
use super::WorldStateSection;
use super::WorldStateUpdate;
use crate::context::AppsInstructions;
use crate::context::ContextualUserFragment;
@@ -41,10 +42,13 @@ impl WorldStateSection for AppsInstructionsState {
|| matches!(previous, PreviousSectionState::Known(previous) if *previous)
|| matches!(previous, PreviousSectionState::Unknown)
{
return (Some(current), None);
return (Some(current), Vec::new());
}
(Some(current), Some(Box::new(AppsInstructions)))
(
Some(current),
vec![WorldStateUpdate::fragment(AppsInstructions)],
)
}
}
@@ -34,7 +34,7 @@ fn legacy_guidance_is_not_injected_again() {
assert!(
world_state
.render_history_diff(/*previous*/ None, &[legacy])
.render_history_fragment_diff(/*previous*/ None, &[legacy])
.1
.is_empty()
);
@@ -49,14 +49,14 @@ fn persisted_guidance_is_restored_only_when_missing_from_history() {
assert_eq!(
world_state
.render_history_diff(Some(&snapshot), &[])
.render_history_fragment_diff(Some(&snapshot), &[])
.1
.len(),
1
);
assert!(
world_state
.render_history_diff(Some(&snapshot), &[retained])
.render_history_fragment_diff(Some(&snapshot), &[retained])
.1
.is_empty()
);
@@ -2,6 +2,7 @@ use super::PreviousSectionState;
use super::SectionTransition;
use super::WorldStateHash;
use super::WorldStateSection;
use super::WorldStateUpdate;
use crate::context::ContextualUserFragment;
use codex_prompts::ResolvedCollaborationModeMessages;
use codex_prompts::ResolvedModelMessages;
@@ -134,14 +135,14 @@ impl WorldStateSection for CollaborationModeState {
}
};
if unchanged {
return (Some(current), None);
return (Some(current), Vec::new());
}
(
Some(current),
Some(Box::new(CollaborationModeInstructions {
vec![WorldStateUpdate::fragment(CollaborationModeInstructions {
instructions: self.instructions.clone().unwrap_or_default(),
})),
})],
)
}
}
@@ -2,6 +2,7 @@ use super::super::PreviousSectionState;
use super::super::test_support::render_section_cases;
use super::*;
use crate::context::world_state::WorldState;
use crate::context::world_state::test_support::FragmentSectionTestExt as _;
use codex_models_manager::model_info::model_info_from_slug;
use codex_prompts::ResolvedCollaborationModeMessages;
use codex_prompts::ResolvedMessage;
@@ -54,7 +55,8 @@ fn instruction_updates_are_applied_once_in_retained_history() {
let expected: ResponseItem = ContextualUserFragment::into(CollaborationModeInstructions {
instructions: instructions.unwrap_or_default().to_string(),
});
let (snapshot, fragments) = world_state.render_history_diff(previous.as_ref(), &history);
let (snapshot, fragments) =
world_state.render_history_fragment_diff(previous.as_ref(), &history);
let updates = fragments
.into_iter()
.map(ContextualUserFragment::into_boxed_response_item)
@@ -65,13 +67,13 @@ fn instruction_updates_are_applied_once_in_retained_history() {
assert!(
world_state
.render_history_diff(previous.as_ref(), &history)
.render_history_fragment_diff(previous.as_ref(), &history)
.1
.is_empty()
);
assert_eq!(
world_state
.render_history_diff(previous.as_ref(), &[])
.render_history_fragment_diff(previous.as_ref(), &[])
.1
.len(),
usize::from(instructions.is_some()),
@@ -124,7 +126,7 @@ fn empty_catalog_collaboration_message_suppresses_legacy_instructions() {
assert_eq!(
state
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.1
.expect("explicit empty collaboration message")
.render(),
@@ -172,7 +174,7 @@ fn legacy_collaboration_mode_snapshots_refresh_catalog_messages_once() {
assert_eq!(
state
.render_diff(PreviousSectionState::Known(&previous))
.render_fragment_diff(PreviousSectionState::Known(&previous))
.1
.expect("legacy snapshot should refresh collaboration instructions")
.render(),
@@ -182,8 +184,11 @@ fn legacy_collaboration_mode_snapshots_refresh_catalog_messages_once() {
);
assert!(
state
.render_diff(PreviousSectionState::Known(
&state.render_diff(PreviousSectionState::Absent).0.unwrap()
.render_fragment_diff(PreviousSectionState::Known(
&state
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap()
))
.1
.is_none()
@@ -1,6 +1,7 @@
use super::PreviousSectionState;
use super::SectionTransition;
use super::WorldStateSection;
use super::WorldStateUpdate;
use crate::context::APPROVED_COMMAND_PREFIX_SAVED_MESSAGE_PREFIX;
use crate::context::ApprovedCommandPrefixSaved;
use crate::context::ContextualUserFragment;
@@ -45,14 +46,17 @@ impl WorldStateSection for CompactPermissionsState {
.cloned()
.collect::<Vec<_>>(),
PreviousSectionState::Absent | PreviousSectionState::Unknown => {
return (Some(current), None);
return (Some(current), Vec::new());
}
};
let fragment: Option<Box<dyn ContextualUserFragment>> =
format_allow_prefixes(added_prefixes)
.filter(|prefixes| !prefixes.is_empty())
.map(|prefixes| Box::new(ApprovedCommandPrefixSaved::new(prefixes)) as _);
(Some(current), fragment)
(
Some(current),
WorldStateUpdate::optional_boxed_fragment(fragment),
)
}
}
@@ -1,4 +1,5 @@
use super::*;
use crate::context::world_state::test_support::FragmentSectionTestExt as _;
use codex_execpolicy::Decision;
use pretty_assertions::assert_eq;
@@ -16,24 +17,24 @@ fn renders_only_newly_approved_prefixes() {
.expect("test prefix should be valid");
let with_new_prefix = CompactPermissionsState::new(&exec_policy);
let existing_snapshot = with_existing_prefix
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap();
let current_snapshot = with_new_prefix
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap();
assert_eq!(
with_new_prefix
.render_diff(Known(&existing_snapshot))
.render_fragment_diff(Known(&existing_snapshot))
.1
.map(|fragment| fragment.render()),
Some("Approved command prefix saved:\n- [\"cargo\", \"test\"]".to_string())
);
assert!(
with_new_prefix
.render_diff(Known(&current_snapshot))
.render_fragment_diff(Known(&current_snapshot))
.1
.is_none()
);
@@ -51,7 +52,7 @@ fn does_not_duplicate_a_retained_legacy_update() {
assert_eq!(
state
.render_diff(Unknown)
.render_fragment_diff(Unknown)
.1
.map(|fragment| fragment.render()),
None
@@ -69,7 +70,7 @@ fn does_not_render_existing_prefixes_without_a_previous_snapshot() {
assert!(
CompactPermissionsState::new(&exec_policy)
.render_diff(Absent)
.render_fragment_diff(Absent)
.1
.is_none()
);
@@ -1,6 +1,7 @@
use super::PreviousSectionState;
use super::SectionTransition;
use super::WorldStateSection;
use super::WorldStateUpdate;
use crate::context::ContextWindowGuidance;
use crate::context::ContextualUserFragment;
@@ -47,7 +48,7 @@ impl WorldStateSection for ContextWindowGuidanceState {
previous: PreviousSectionState<'_, Self::Snapshot>,
) -> SectionTransition<Self::Snapshot> {
if matches!(previous, PreviousSectionState::Known(previous) if previous == &self.message) {
return (None, None);
return (None, Vec::new());
}
let previous_may_contain_guidance = match previous {
PreviousSectionState::Known(previous) => !previous.is_empty(),
@@ -56,7 +57,7 @@ impl WorldStateSection for ContextWindowGuidanceState {
};
let message = if self.message.is_empty() {
if !previous_may_contain_guidance {
return (Some(self.message.clone()), None);
return (Some(self.message.clone()), Vec::new());
}
REMOVAL_NOTICE.to_string()
} else if previous_may_contain_guidance {
@@ -66,7 +67,9 @@ impl WorldStateSection for ContextWindowGuidanceState {
};
(
Some(self.message.clone()),
Some(Box::new(ContextWindowGuidance::new(&message))),
vec![WorldStateUpdate::fragment(ContextWindowGuidance::new(
&message,
))],
)
}
}
@@ -35,7 +35,7 @@ fn guidance_transitions_render_once(
.into_iter()
.collect();
let (next_snapshot, fragments) =
refreshed.render_history_diff(Some(&original.render_full().0), &history);
refreshed.render_history_fragment_diff(Some(&original.render_full().0), &history);
assert_eq!(
fragments
.iter()
@@ -56,7 +56,7 @@ fn guidance_transitions_render_once(
.map(ContextualUserFragment::into_boxed_response_item),
);
let (repeated_snapshot, repeated_fragments) =
refreshed.render_history_diff(Some(&snapshot), &history);
refreshed.render_history_fragment_diff(Some(&snapshot), &history);
assert_eq!(repeated_snapshot, snapshot);
assert!(repeated_fragments.is_empty());
}
@@ -83,7 +83,7 @@ fn legacy_guidance_is_reconciled_once(
let retained: ResponseItem =
ContextualUserFragment::into(ContextWindowGuidance::new("previous guidance"));
let fragments = state
.render_history_diff(snapshot.as_ref(), std::slice::from_ref(&retained))
.render_history_fragment_diff(snapshot.as_ref(), std::slice::from_ref(&retained))
.1;
assert_eq!(
fragments
@@ -104,7 +104,7 @@ fn legacy_guidance_is_reconciled_once(
}
assert!(
state
.render_history_diff(Some(&state.render_full().0), &history)
.render_history_fragment_diff(Some(&state.render_full().0), &history)
.1
.is_empty()
);
@@ -1,6 +1,7 @@
use super::PreviousSectionState;
use super::SectionTransition;
use super::WorldStateSection;
use super::WorldStateUpdate;
use crate::context::ContextualUserFragment;
use crate::context::environment_context::FileSystemContext;
use crate::context::environment_context::NetworkContext;
@@ -189,7 +190,10 @@ impl WorldStateSection for EnvironmentsState {
subagents: self.subagents.clone(),
}) as Box<dyn ContextualUserFragment>
});
(Some(current), fragment)
(
Some(current),
WorldStateUpdate::optional_boxed_fragment(fragment),
)
}
}
@@ -1,3 +1,4 @@
use crate::context::world_state::test_support::FragmentSectionTestExt as _;
use crate::shell::ShellType;
use super::*;
@@ -368,11 +369,14 @@ fn powershell_environment() -> EnvironmentsState {
#[test]
fn shell_version_diff_restates_shell_from_legacy_snapshot() {
let current = powershell_environment();
let mut previous = current.render_diff(PreviousSectionState::Absent).0.unwrap();
let mut previous = current
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap();
previous.shell_version = None;
previous.environments.get_mut("local").expect("local").shell = None;
let rendered = current
.render_diff(PreviousSectionState::Known(&previous))
.render_fragment_diff(PreviousSectionState::Known(&previous))
.1
.expect("shell version update")
.render();
@@ -391,9 +395,9 @@ fn shell_version_diff_clears_previously_visible_version() {
};
assert_eq!(
current
.render_diff(PreviousSectionState::Known(
.render_fragment_diff(PreviousSectionState::Known(
&previous
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap()
))
@@ -413,9 +417,9 @@ fn current_date_diff_clears_once_and_recovers() {
let unavailable = EnvironmentsState::default();
assert_eq!(
unavailable
.render_diff(PreviousSectionState::Known(
.render_fragment_diff(PreviousSectionState::Known(
&available
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap()
))
@@ -426,9 +430,9 @@ fn current_date_diff_clears_once_and_recovers() {
);
assert!(
unavailable
.render_diff(PreviousSectionState::Known(
.render_fragment_diff(PreviousSectionState::Known(
&unavailable
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap()
))
@@ -437,9 +441,9 @@ fn current_date_diff_clears_once_and_recovers() {
);
assert_eq!(
available
.render_diff(PreviousSectionState::Known(
.render_fragment_diff(PreviousSectionState::Known(
&unavailable
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap()
))
@@ -1,6 +1,7 @@
use super::super::PreviousSectionState;
use super::super::test_support::render_section_cases;
use super::*;
use crate::context::world_state::test_support::FragmentSectionTestExt as _;
use anyhow::Result;
use codex_exec_server::LOCAL_ENVIRONMENT_ID;
use codex_protocol::models::PermissionProfile;
@@ -173,9 +174,12 @@ fn changing_primary_environment_updates_model_context_and_persisted_state() -> R
.collect(),
..Default::default()
};
let previous = before.render_diff(PreviousSectionState::Absent).0.unwrap();
let previous = before
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap();
let rendered = after
.render_diff(PreviousSectionState::Known(&previous))
.render_fragment_diff(PreviousSectionState::Known(&previous))
.1
.expect("primary change should update the model")
.render();
@@ -232,14 +236,14 @@ fn legacy_single_environment_snapshot_does_not_change() -> Result<()> {
assert!(
environment
.render_diff(PreviousSectionState::Known(&legacy_snapshot))
.render_fragment_diff(PreviousSectionState::Known(&legacy_snapshot))
.1
.is_none()
);
assert_eq!(
serde_json::to_value(
environment
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap()
)?["environments"]["local"],
@@ -288,8 +292,11 @@ fn crossing_single_environment_boundary_restates_current_environments() -> Resul
};
let expanded = multiple
.render_diff(PreviousSectionState::Known(
&single.render_diff(PreviousSectionState::Absent).0.unwrap(),
.render_fragment_diff(PreviousSectionState::Known(
&single
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap(),
))
.1
.expect("adding an environment should update the model")
@@ -302,9 +309,9 @@ fn crossing_single_environment_boundary_restates_current_environments() -> Resul
);
let reduced = single
.render_diff(PreviousSectionState::Known(
.render_fragment_diff(PreviousSectionState::Known(
&multiple
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap(),
))
@@ -378,8 +385,11 @@ fn failure_context_is_escaped_incremental_and_cleared_on_recovery() -> Result<()
"#);
assert!(
failed
.render_diff(PreviousSectionState::Known(
&failed.render_diff(PreviousSectionState::Absent).0.unwrap()
.render_fragment_diff(PreviousSectionState::Known(
&failed
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap()
))
.1
.is_none()
@@ -394,8 +404,11 @@ fn failure_context_is_escaped_incremental_and_cleared_on_recovery() -> Result<()
..Default::default()
};
let update = recovered
.render_diff(PreviousSectionState::Known(
&failed.render_diff(PreviousSectionState::Absent).0.unwrap(),
.render_fragment_diff(PreviousSectionState::Known(
&failed
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap(),
))
.1
.expect("recovery must be visible to the model");
@@ -1,6 +1,7 @@
use super::PreviousSectionState;
use super::SectionTransition;
use super::WorldStateSection;
use super::WorldStateUpdate;
use crate::context::ContextualUserFragment;
use crate::context::EnvironmentsInstructions;
@@ -41,10 +42,13 @@ impl WorldStateSection for EnvironmentsInstructionsState {
|| matches!(previous, PreviousSectionState::Known(previous) if *previous)
|| matches!(previous, PreviousSectionState::Unknown)
{
return (Some(current), None);
return (Some(current), Vec::new());
}
(Some(current), Some(Box::new(EnvironmentsInstructions)))
(
Some(current),
vec![WorldStateUpdate::fragment(EnvironmentsInstructions)],
)
}
}
@@ -33,7 +33,7 @@ fn legacy_guidance_is_not_injected_again() {
assert!(
world_state
.render_history_diff(/*previous*/ None, &[legacy])
.render_history_fragment_diff(/*previous*/ None, &[legacy])
.1
.is_empty()
);
@@ -48,14 +48,14 @@ fn persisted_guidance_is_restored_only_when_missing_from_history() {
assert_eq!(
world_state
.render_history_diff(Some(&snapshot), &[])
.render_history_fragment_diff(Some(&snapshot), &[])
.1
.len(),
1
);
assert!(
world_state
.render_history_diff(Some(&snapshot), &[retained])
.render_history_fragment_diff(Some(&snapshot), &[retained])
.1
.is_empty()
);
@@ -2,6 +2,7 @@ use super::PreviousSectionState;
use super::SectionTransition;
use super::WorldStateHash;
use super::WorldStateSection;
use super::WorldStateUpdate;
use crate::context::ContextualUserFragment;
use codex_config::Sourced;
use codex_protocol::models::ContentItemKind;
@@ -31,10 +32,6 @@ impl ContextualUserFragment for ManagedDeveloperInstructions {
"developer"
}
fn requires_separate_message(&self) -> bool {
true
}
fn markers(&self) -> (&'static str, &'static str) {
Self::type_markers()
}
@@ -128,7 +125,7 @@ impl WorldStateSection for ManagedDeveloperInstructionsState {
.map(WorldStateHash::from_fragment),
};
if matches!(previous, PreviousSectionState::Known(previous) if previous == &current) {
return (None, None);
return (None, Vec::new());
}
let previous_had_instructions = match previous {
PreviousSectionState::Absent => false,
@@ -143,9 +140,12 @@ impl WorldStateSection for ManagedDeveloperInstructionsState {
(None, true) => ManagedDeveloperInstructions {
instructions: REMOVAL_NOTICE.to_string(),
},
(None, false) => return (Some(current), None),
(None, false) => return (Some(current), Vec::new()),
};
(Some(current), Some(Box::new(fragment)))
(
Some(current),
vec![WorldStateUpdate::fragment(fragment).standalone()],
)
}
}
@@ -23,7 +23,8 @@ fn managed_policy_updates_and_removal_are_emitted_once() {
instructions.map(|text| Sourced::new(text.to_string(), RequirementSource::Unknown));
let mut world_state = WorldState::default();
world_state.add_section(ManagedDeveloperInstructionsState::new(requirement.as_ref()));
let (snapshot, fragments) = world_state.render_history_diff(previous.as_ref(), &history);
let (snapshot, fragments) =
world_state.render_history_fragment_diff(previous.as_ref(), &history);
let updates = fragments
.into_iter()
.map(ContextualUserFragment::into_boxed_response_item)
@@ -41,7 +42,7 @@ fn managed_policy_updates_and_removal_are_emitted_once() {
previous = Some(snapshot);
assert!(
world_state
.render_history_diff(previous.as_ref(), &history)
.render_history_fragment_diff(previous.as_ref(), &history)
.1
.is_empty()
);
@@ -63,7 +64,10 @@ fn managed_policy_updates_and_removal_are_emitted_once() {
world_state.add_section(ManagedDeveloperInstructionsState::new(requirement.as_ref()));
assert_eq!(
world_state
.render_history_diff(/*previous*/ None, std::slice::from_ref(&legacy_policy))
.render_history_fragment_diff(
/*previous*/ None,
std::slice::from_ref(&legacy_policy)
)
.1
.into_iter()
.map(ContextualUserFragment::into_boxed_response_item)
+102 -22
View File
@@ -15,7 +15,7 @@ mod persistent_mode;
mod plugins_instructions;
mod realtime;
#[cfg(test)]
mod test_support;
pub(crate) mod test_support;
mod tools;
use crate::context::ContextualUserFragment;
@@ -56,7 +56,86 @@ pub(crate) use plugins_instructions::PluginsInstructionsState;
pub(crate) use realtime::RealtimeState;
pub(crate) use tools::ToolsState;
pub(crate) type SectionTransition<S = Value> = (Option<S>, Option<Box<dyn ContextualUserFragment>>);
/// One contribution to model context and its placement policy.
pub(crate) struct WorldStateUpdate {
pub(crate) placement: Placement,
pub(crate) content: WorldStateUpdateContent,
}
#[derive(Clone, Copy, PartialEq, Eq)]
pub(crate) enum Placement {
/// A separate item placed at the front when assembling initial context.
#[cfg_attr(
not(test),
expect(dead_code, reason = "Prefix producers are added separately")
)]
Prefix,
Standalone,
Mergeable,
}
pub(crate) enum WorldStateUpdateContent {
Fragment(Box<dyn ContextualUserFragment>),
#[cfg_attr(
not(test),
expect(dead_code, reason = "Item producers are added separately")
)]
Item(Box<ResponseItem>),
}
impl WorldStateUpdate {
pub(crate) fn fragment(fragment: impl ContextualUserFragment + 'static) -> Self {
Self::boxed_fragment(Box::new(fragment))
}
pub(crate) fn boxed_fragment(fragment: Box<dyn ContextualUserFragment>) -> Self {
Self {
placement: Placement::Mergeable,
content: WorldStateUpdateContent::Fragment(fragment),
}
}
pub(crate) fn optional_boxed_fragment(
fragment: Option<Box<dyn ContextualUserFragment>>,
) -> Vec<Self> {
fragment.into_iter().map(Self::boxed_fragment).collect()
}
pub(crate) fn optional_standalone_boxed_fragment(
fragment: Option<Box<dyn ContextualUserFragment>>,
) -> Vec<Self> {
fragment
.into_iter()
.map(Self::boxed_fragment)
.map(Self::standalone)
.collect()
}
pub(crate) fn standalone(mut self) -> Self {
self.placement = Placement::Standalone;
self
}
}
pub(crate) type SectionTransition<S = Value> = (Option<S>, Vec<WorldStateUpdate>);
/// Separates the rendered window prefix from the remaining initial-context updates.
pub(crate) fn split_prefix_updates(
updates: Vec<WorldStateUpdate>,
) -> (Vec<ResponseItem>, Vec<WorldStateUpdate>) {
let mut prefix = Vec::new();
let mut context = Vec::new();
for update in updates {
match update.placement {
Placement::Prefix => prefix.push(match update.content {
WorldStateUpdateContent::Item(item) => *item,
WorldStateUpdateContent::Fragment(fragment) => fragment.into_boxed_response_item(),
}),
Placement::Standalone | Placement::Mergeable => context.push(update),
}
}
(prefix, context)
}
trait ErasedWorldStateSection: Send + Sync {
fn matches_legacy_fragment(&self, role: &str, text: &str) -> bool;
@@ -130,10 +209,10 @@ impl<S: WorldStateSection> ErasedWorldStateSection for S {
PreviousSectionState::Absent => PreviousSectionState::Absent,
PreviousSectionState::Unknown => PreviousSectionState::Unknown,
};
let (snapshot, fragment) = WorldStateSection::render_diff(self, previous);
let (snapshot, updates) = WorldStateSection::render_diff(self, previous);
(
snapshot.map(|snapshot| section_snapshot(self, snapshot).unwrap_or(Value::Null)),
fragment,
updates,
)
}
}
@@ -164,13 +243,16 @@ impl ErasedWorldStateSection for ExtensionWorldStateSection {
remove_null_object_fields(&mut snapshot);
snapshot
});
let fragment = fragment.map(|fragment| {
Box::new(WorldStateContextFragment {
fragment,
content_kind: ContentItemKind(format!("{}.instructions", self.0.id())),
}) as _
});
(snapshot, fragment)
let updates = fragment
.into_iter()
.map(|fragment| {
WorldStateUpdate::fragment(WorldStateContextFragment {
fragment,
content_kind: ContentItemKind(format!("{}.instructions", self.0.id())),
})
})
.collect();
(snapshot, updates)
}
}
@@ -248,7 +330,7 @@ pub(crate) trait WorldStateSection: Send + Sync + 'static {
false
}
/// Returns independently optional updates to the snapshot and model context.
/// Returns an optional snapshot update and ordered model-context updates.
/// A missing snapshot leaves the stored comparison state unchanged.
fn render_diff(
&self,
@@ -377,8 +459,8 @@ impl WorldState {
}
}
/// Renders every section as new, without any known previous state.
pub(crate) fn render_full(&self) -> (WorldStateSnapshot, Vec<Box<dyn ContextualUserFragment>>) {
/// Renders every section as new, preserving update order and placement.
pub(crate) fn render_full(&self) -> (WorldStateSnapshot, Vec<WorldStateUpdate>) {
self.render_with(/*stored*/ None, |_, _| PreviousSectionState::Absent)
}
@@ -387,7 +469,7 @@ impl WorldState {
&self,
previous: Option<&WorldStateSnapshot>,
items: impl IntoIterator<Item = &'a ResponseItem> + Clone,
) -> (WorldStateSnapshot, Vec<Box<dyn ContextualUserFragment>>) {
) -> (WorldStateSnapshot, Vec<WorldStateUpdate>) {
self.render_with(previous, |id, section| {
if let Some(previous) = previous.and_then(|previous| previous.sections.get(id)) {
if section.has_retained_fragment_matcher()
@@ -409,12 +491,12 @@ impl WorldState {
&self,
stored: Option<&WorldStateSnapshot>,
mut previous: impl FnMut(&str, &dyn ErasedWorldStateSection) -> PreviousSectionState<'a, Value>,
) -> (WorldStateSnapshot, Vec<Box<dyn ContextualUserFragment>>) {
) -> (WorldStateSnapshot, Vec<WorldStateUpdate>) {
let mut snapshot = WorldStateSnapshot::default();
let mut fragments = Vec::new();
let mut updates = Vec::new();
for (id, section) in &self.sections {
let prior = previous(id, section.as_ref());
let (section_snapshot, fragment) = section.render_diff(prior);
let (section_snapshot, section_updates) = section.render_diff(prior);
// A skipped snapshot retains stored state even if history required a fresh render.
let section_snapshot = section_snapshot.or_else(|| {
stored
@@ -428,11 +510,9 @@ impl WorldState {
.sections
.insert((*id).to_string(), section_snapshot);
}
if let Some(fragment) = fragment {
fragments.push(fragment);
}
updates.extend(section_updates);
}
(snapshot, fragments)
(snapshot, updates)
}
}
@@ -1,6 +1,7 @@
use super::PreviousSectionState;
use super::SectionTransition;
use super::WorldStateSection;
use super::WorldStateUpdate;
use crate::context::ContextualUserFragment;
use crate::context::ModelSwitchInstructions;
@@ -56,7 +57,10 @@ impl WorldStateSection for ModelInstructionsState {
Box::new(ModelSwitchInstructions::new(self.instructions.clone()))
as Box<dyn ContextualUserFragment>
});
(Some(current), fragment)
(
Some(current),
WorldStateUpdate::optional_standalone_boxed_fragment(fragment),
)
}
}
@@ -6,6 +6,7 @@ use super::PreviousSectionState;
use super::SectionTransition;
use super::WorldStateHash;
use super::WorldStateSection;
use super::WorldStateUpdate;
use crate::agent::child_config::MAX_SPAWN_AGENT_MODEL_OVERRIDES;
use crate::agent::child_config::model_supports_multi_agent_backend;
use crate::context::ContextualUserFragment;
@@ -135,8 +136,11 @@ impl WorldStateSection for ModelCatalogState {
if matches!(previous, PreviousSectionState::Known(previous) if previous == &current)
|| self.catalog.is_empty() && matches!(previous, PreviousSectionState::Absent)
{
return (Some(current), None);
return (Some(current), Vec::new());
}
(Some(current), Some(Box::new(self.clone())))
(
Some(current),
vec![WorldStateUpdate::fragment(self.clone())],
)
}
}
@@ -1,4 +1,5 @@
use super::*;
use crate::context::world_state::test_support::FragmentSectionTestExt as _;
use pretty_assertions::assert_eq;
#[test]
@@ -13,7 +14,7 @@ fn model_change_renders_when_persisted_or_inferred_from_previous_turn() {
] {
assert_eq!(
state
.render_diff(previous)
.render_fragment_diff(previous)
.1
.expect("model change should render")
.markers(),
@@ -29,9 +30,14 @@ fn unchanged_model_does_not_render() {
assert!(
state
.render_diff(PreviousSectionState::Known(&previous))
.render_fragment_diff(PreviousSectionState::Known(&previous))
.1
.is_none()
);
assert!(
state
.render_fragment_diff(PreviousSectionState::Absent)
.1
.is_none()
);
assert!(state.render_diff(PreviousSectionState::Absent).1.is_none());
}
@@ -2,6 +2,7 @@ use super::PreviousSectionState;
use super::SectionTransition;
use super::WorldStateHash;
use super::WorldStateSection;
use super::WorldStateUpdate;
use super::multi_agent_usage_hint::MultiAgentUsageHintState;
use crate::context::ContextualUserFragment;
use crate::context::multi_agent_mode_instructions::MultiAgentModeInstructions;
@@ -66,7 +67,7 @@ impl WorldStateSection for MultiAgentModeState {
if previous.mode.as_ref() == Some(mode)
&& previous.usage_hint_hash == self.usage_hint_hash =>
{
return (None, None);
return (None, Vec::new());
}
(Some(mode), _) => mode.clone(),
(None, PreviousSectionState::Known(previous))
@@ -76,13 +77,16 @@ impl WorldStateSection for MultiAgentModeState {
}
(None, PreviousSectionState::Unknown) => MultiAgentMode::ExplicitRequestOnly,
(None, PreviousSectionState::Absent | PreviousSectionState::Known(_)) => {
return (Some(self.clone()), None);
return (Some(self.clone()), Vec::new());
}
};
let fragment = MultiAgentModeInstructions::from_mode(mode)
.map(|instructions| Box::new(instructions) as Box<dyn ContextualUserFragment>);
(Some(self.clone()), fragment)
(
Some(self.clone()),
WorldStateUpdate::optional_boxed_fragment(fragment),
)
}
}
@@ -2,6 +2,7 @@ use super::super::test_support::render_section_cases;
use super::*;
use crate::context::MultiAgentRoleInstructions;
use crate::context::world_state::WorldState;
use crate::context::world_state::test_support::FragmentSectionTestExt as _;
use codex_protocol::models::ResponseItem;
use codex_utils_output_truncation::approx_token_count;
use pretty_assertions::assert_eq;
@@ -52,21 +53,21 @@ fn persisted_mode_is_restored_only_when_missing_from_history() {
assert_eq!(
world_state
.render_history_diff(/*previous*/ None, std::slice::from_ref(&retained))
.render_history_fragment_diff(/*previous*/ None, std::slice::from_ref(&retained))
.1
.len(),
1,
);
assert_eq!(
world_state
.render_history_diff(Some(&snapshot), &[])
.render_history_fragment_diff(Some(&snapshot), &[])
.1
.len(),
1
);
assert!(
world_state
.render_history_diff(Some(&snapshot), &[retained])
.render_history_fragment_diff(Some(&snapshot), &[retained])
.1
.is_empty()
);
@@ -83,7 +84,7 @@ fn unchanged_mode_is_reemitted_after_usage_hint_migration() {
);
let instructions = current
.render_diff(PreviousSectionState::Known(&previous))
.render_fragment_diff(PreviousSectionState::Known(&previous))
.1
.expect("unchanged mode should follow migrated usage instructions");
@@ -111,7 +112,7 @@ fn catalog_role_updates_remain_separate_from_active_mode() {
let mut previous = WorldState::default();
previous.add_section(previous_hint);
previous.add_section(previous_mode);
let (previous_snapshot, previous_fragments) = previous.render_full();
let (previous_snapshot, previous_fragments) = previous.render_full_fragments();
let history = previous_fragments
.into_iter()
.map(ContextualUserFragment::into_boxed_response_item)
@@ -125,7 +126,7 @@ fn catalog_role_updates_remain_separate_from_active_mode() {
current.add_section(current_hint);
current.add_section(current_mode);
let updates = crate::context_manager::updates::merge_contextual_fragments(
let updates = crate::context_manager::updates::merge_world_state_updates(
current
.render_history_diff(Some(&previous_snapshot), &history)
.1,
@@ -145,7 +146,7 @@ fn catalog_role_updates_remain_separate_from_active_mode() {
fn custom_mode_is_bounded_before_snapshot_and_rendering() {
let state = state(Some(MultiAgentMode::Custom("custom mode ".repeat(1_000))));
let Some(MultiAgentMode::Custom(snapshot_mode)) = state
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap()
.mode
@@ -155,7 +156,7 @@ fn custom_mode_is_bounded_before_snapshot_and_rendering() {
assert!(approx_token_count(&snapshot_mode) < 1_000);
let rendered = state
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.1
.expect("custom mode should render")
.render();
@@ -2,6 +2,7 @@ use super::PreviousSectionState;
use super::SectionTransition;
use super::WorldStateHash;
use super::WorldStateSection;
use super::WorldStateUpdate;
use crate::context::ContextualUserFragment;
use crate::context::MultiAgentRoleInstructions;
use crate::context::MultiAgentUsageHint;
@@ -37,7 +38,7 @@ impl WorldStateSection for MultiAgentUsageHintState {
) -> SectionTransition<Self::Snapshot> {
let fragment: Option<Box<dyn ContextualUserFragment>> = match previous {
PreviousSectionState::Known(previous) if previous == &self.fingerprint => {
return (None, None);
return (None, Vec::new());
}
PreviousSectionState::Unknown => None,
PreviousSectionState::Known(_) | PreviousSectionState::Absent => {
@@ -50,6 +51,9 @@ impl WorldStateSection for MultiAgentUsageHintState {
}
}
};
(Some(self.fingerprint.clone()), fragment)
(
Some(self.fingerprint.clone()),
WorldStateUpdate::optional_standalone_boxed_fragment(fragment),
)
}
}
@@ -4,6 +4,7 @@ use super::PreviousSectionState;
use super::SectionTransition;
use super::WorldStateHash;
use super::WorldStateSection;
use super::WorldStateUpdate;
use crate::context::ApprovedCommandPrefixSaved;
use crate::context::ContextualUserFragment;
use crate::context::PermissionsInstructions;
@@ -105,7 +106,7 @@ impl WorldStateSection for PermissionsState {
},
) if previous_instructions == instructions => {
if previous_prefixes == approved_command_prefixes {
return (None, None);
return (None, Vec::new());
}
if previous_prefixes.is_subset(approved_command_prefixes) {
let added_prefixes = approved_command_prefixes
@@ -115,7 +116,9 @@ impl WorldStateSection for PermissionsState {
if let Some(prefixes) = format_allow_prefixes(added_prefixes) {
return (
Some(current),
Some(Box::new(ApprovedCommandPrefixSaved::new(prefixes))),
vec![WorldStateUpdate::fragment(ApprovedCommandPrefixSaved::new(
prefixes,
))],
);
}
}
@@ -124,12 +127,15 @@ impl WorldStateSection for PermissionsState {
PreviousSectionState::Known(PermissionsSnapshot::Legacy(previous)),
PermissionsSnapshot::Current { .. },
) if previous == &WorldStateHash::from_fragment(&self.instructions) => {
return (Some(current), None);
return (Some(current), Vec::new());
}
_ => {}
}
(Some(current), Some(Box::new(self.instructions.clone())))
(
Some(current),
vec![WorldStateUpdate::fragment(self.instructions.clone())],
)
}
}
@@ -1,4 +1,5 @@
use super::*;
use crate::context::world_state::test_support::FragmentSectionTestExt as _;
use crate::context::world_state::test_support::render_section_cases;
use codex_execpolicy::Decision;
use codex_models_manager::model_info::model_info_from_slug;
@@ -50,15 +51,15 @@ fn approved_prefix_is_rendered_without_reinjecting_permissions() {
let with_approved_prefix = permissions_state_with_default_messages(&exec_policy);
let approved_prefix = r#"["touch", "allow-prefix.txt"]"#;
let without_snapshot = without_approved_prefix
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap();
let with_snapshot = with_approved_prefix
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap();
let rendered_update = with_approved_prefix
.render_diff(Known(&without_snapshot))
.render_fragment_diff(Known(&without_snapshot))
.1
.expect("approving a prefix should render a world-state update")
.render();
@@ -113,22 +114,22 @@ fn renders_only_newly_approved_prefixes() {
.expect("test prefix should be valid");
let with_new_prefix = permissions_state_with_default_messages(&exec_policy);
let existing_snapshot = with_existing_prefix
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap();
let current_snapshot = with_new_prefix
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap();
assert_eq!(
with_new_prefix
.render_diff(Known(&existing_snapshot))
.render_fragment_diff(Known(&existing_snapshot))
.1
.map(|fragment| fragment.render()),
Some("Approved command prefix saved:\n- [\"cargo\", \"test\"]".to_string())
);
let (snapshot, fragment) = with_new_prefix.render_diff(Known(&current_snapshot));
let (snapshot, fragment) = with_new_prefix.render_fragment_diff(Known(&current_snapshot));
assert_eq!(
(snapshot, fragment.map(|fragment| fragment.render())),
(None, None)
@@ -161,13 +162,13 @@ fn legacy_snapshot_deserializes_and_only_suppresses_matching_full_permissions()
assert!(
world_state
.render_history_diff(Some(&matching_legacy), std::slice::from_ref(&retained))
.render_history_fragment_diff(Some(&matching_legacy), std::slice::from_ref(&retained))
.1
.is_empty()
);
assert_eq!(
world_state
.render_history_diff(Some(&stale_legacy), &[retained])
.render_history_fragment_diff(Some(&stale_legacy), &[retained])
.1
.into_iter()
.map(|fragment| fragment.render())
@@ -188,9 +189,9 @@ fn removing_an_approved_prefix_renders_full_permissions() {
let without_approved_prefix = permissions_state_with_default_messages(&Policy::empty());
let rendered = without_approved_prefix
.render_diff(Known(
.render_fragment_diff(Known(
&with_approved_prefix
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.0
.unwrap(),
))
@@ -221,21 +222,21 @@ fn persisted_permissions_are_detected_inside_bundled_developer_messages() {
assert_eq!(
world_state
.render_history_diff(/*previous*/ None, std::slice::from_ref(&retained))
.render_history_fragment_diff(/*previous*/ None, std::slice::from_ref(&retained))
.1
.len(),
1,
);
assert_eq!(
world_state
.render_history_diff(Some(&snapshot), &[])
.render_history_fragment_diff(Some(&snapshot), &[])
.1
.len(),
1,
);
assert!(
world_state
.render_history_diff(Some(&snapshot), &[bundled_retained])
.render_history_fragment_diff(Some(&snapshot), &[bundled_retained])
.1
.is_empty()
);
@@ -5,6 +5,7 @@ use super::PreviousSectionState;
use super::SectionTransition;
use super::WorldStateHash;
use super::WorldStateSection;
use super::WorldStateUpdate;
use crate::context::ContextualUserFragment;
use codex_protocol::models::ContentItemKind;
use serde::Deserialize;
@@ -94,7 +95,7 @@ impl WorldStateSection for PersistentModeState {
.then(|| WorldStateHash::from_fragment(self)),
};
if matches!(previous, PreviousSectionState::Known(previous) if previous == &current) {
return (None, None);
return (None, Vec::new());
}
let previous_had_instructions = match previous {
PreviousSectionState::Absent => false,
@@ -102,12 +103,15 @@ impl WorldStateSection for PersistentModeState {
PreviousSectionState::Known(previous) => previous.instructions.is_some(),
};
let instructions = match (self.instructions.as_str(), previous_had_instructions) {
("", false) => return (Some(current), None),
("", false) => return (Some(current), Vec::new()),
("", true) => REMOVAL_NOTICE.to_string(),
(instructions, true) => format!("{REPLACEMENT_NOTICE}\n\n{instructions}"),
(instructions, false) => instructions.to_string(),
};
(Some(current), Some(Box::new(Self { instructions })))
(
Some(current),
vec![WorldStateUpdate::fragment(Self { instructions })],
)
}
}
@@ -27,7 +27,8 @@ fn persistent_instructions_follow_mode_and_catalog_updates_without_duplicates()
instructions,
/*send_user_message_async_available*/ false,
));
let (snapshot, fragments) = world_state.render_history_diff(previous.as_ref(), &history);
let (snapshot, fragments) =
world_state.render_history_fragment_diff(previous.as_ref(), &history);
let updates = fragments
.into_iter()
.map(ContextualUserFragment::into_boxed_response_item)
@@ -68,7 +69,10 @@ fn retained_persistent_instructions_are_replaced_or_retired_without_a_snapshot()
));
assert_eq!(
world_state
.render_history_diff(/*previous*/ None, std::slice::from_ref(&retained))
.render_history_fragment_diff(
/*previous*/ None,
std::slice::from_ref(&retained)
)
.1
.into_iter()
.map(ContextualUserFragment::into_boxed_response_item)
@@ -1,6 +1,7 @@
use super::PreviousSectionState;
use super::SectionTransition;
use super::WorldStateSection;
use super::WorldStateUpdate;
use crate::context::AvailablePluginsInstructions;
use crate::context::ContextualUserFragment;
@@ -41,10 +42,13 @@ impl WorldStateSection for PluginsInstructionsState {
|| matches!(previous, PreviousSectionState::Known(previous) if *previous)
|| matches!(previous, PreviousSectionState::Unknown)
{
return (Some(current), None);
return (Some(current), Vec::new());
}
(Some(current), Some(Box::new(AvailablePluginsInstructions)))
(
Some(current),
vec![WorldStateUpdate::fragment(AvailablePluginsInstructions)],
)
}
}
@@ -34,7 +34,7 @@ fn legacy_guidance_is_not_injected_again() {
assert!(
world_state
.render_history_diff(/*previous*/ None, &[legacy])
.render_history_fragment_diff(/*previous*/ None, &[legacy])
.1
.is_empty()
);
@@ -49,14 +49,14 @@ fn persisted_guidance_is_restored_only_when_missing_from_history() {
assert_eq!(
world_state
.render_history_diff(Some(&snapshot), &[])
.render_history_fragment_diff(Some(&snapshot), &[])
.1
.len(),
1
);
assert!(
world_state
.render_history_diff(Some(&snapshot), &[retained])
.render_history_fragment_diff(Some(&snapshot), &[retained])
.1
.is_empty()
);
@@ -1,6 +1,7 @@
use super::PreviousSectionState;
use super::SectionTransition;
use super::WorldStateSection;
use super::WorldStateUpdate;
use crate::context::ContextualUserFragment;
use crate::context::RealtimeEndInstructions;
use crate::context::RealtimeStartInstructions;
@@ -77,7 +78,7 @@ impl WorldStateSection for RealtimeState {
) -> SectionTransition<Self::Snapshot> {
let fragment = match previous {
PreviousSectionState::Known(previous) if previous == &self.snapshot => {
return (None, None);
return (None, Vec::new());
}
PreviousSectionState::Known(previous) => self.render_transition(previous.active),
PreviousSectionState::Absent | PreviousSectionState::Unknown
@@ -87,7 +88,10 @@ impl WorldStateSection for RealtimeState {
}
PreviousSectionState::Absent | PreviousSectionState::Unknown => None,
};
(Some(self.snapshot.clone()), fragment)
(
Some(self.snapshot.clone()),
WorldStateUpdate::optional_boxed_fragment(fragment),
)
}
}
@@ -1,7 +1,12 @@
use super::ErasedWorldStateSection;
use super::PreviousSectionState;
use super::WorldState;
use super::WorldStateSection;
use super::WorldStateSnapshot;
use super::WorldStateUpdate;
use super::WorldStateUpdateContent;
use crate::context::ContextualUserFragment;
use codex_protocol::models::ResponseItem;
pub(super) fn render_section_cases<'a, S: WorldStateSection>(
cases: &[(PreviousSectionState<'a, S>, PreviousSectionState<'a, S>)],
@@ -50,7 +55,9 @@ fn render_diff<S: WorldStateSection>(
PreviousSectionState::Known(&previous_snapshot)
}
};
ErasedWorldStateSection::render_diff(*after, previous).1
let mut fragments = expect_fragments(ErasedWorldStateSection::render_diff(*after, previous).1);
assert!(fragments.len() <= 1, "expected at most one fragment");
fragments.pop()
}
fn render_snapshot<S: WorldStateSection>(section: &S) -> String {
@@ -82,3 +89,61 @@ fn snapshot_value<S: WorldStateSection>(section: &S) -> serde_json::Value {
.0
.expect("world-state section snapshot should serialize to a non-null value")
}
/// Keeps text-only section assertions explicit when a section gains item output.
pub(crate) fn expect_fragments(
updates: Vec<WorldStateUpdate>,
) -> Vec<Box<dyn ContextualUserFragment>> {
updates
.into_iter()
.map(|update| expect_fragment(update.content))
.collect()
}
fn expect_fragment(content: WorldStateUpdateContent) -> Box<dyn ContextualUserFragment> {
match content {
WorldStateUpdateContent::Fragment(fragment) => fragment,
WorldStateUpdateContent::Item(item) => panic!("expected a context fragment, got {item:?}"),
}
}
/// Renders a section that is expected to emit at most one contextual fragment.
pub(crate) trait FragmentSectionTestExt: WorldStateSection {
fn render_fragment_diff(
&self,
previous: PreviousSectionState<'_, Self::Snapshot>,
) -> (
Option<Self::Snapshot>,
Option<Box<dyn ContextualUserFragment>>,
) {
let (snapshot, updates) = self.render_diff(previous);
let mut fragments = expect_fragments(updates);
assert!(fragments.len() <= 1, "expected at most one fragment");
(snapshot, fragments.pop())
}
}
impl<S: WorldStateSection> FragmentSectionTestExt for S {}
impl WorldState {
pub(crate) fn render_full_fragments(
&self,
) -> (WorldStateSnapshot, Vec<Box<dyn ContextualUserFragment>>) {
let (snapshot, updates) = self.render_full();
let (prefix, context) = super::split_prefix_updates(updates);
assert!(
prefix.is_empty(),
"expected fragments, got prefix {prefix:?}"
);
(snapshot, expect_fragments(context))
}
pub(crate) fn render_history_fragment_diff<'a>(
&self,
previous: Option<&WorldStateSnapshot>,
items: impl IntoIterator<Item = &'a ResponseItem> + Clone,
) -> (WorldStateSnapshot, Vec<Box<dyn ContextualUserFragment>>) {
let (snapshot, updates) = self.render_history_diff(previous, items);
(snapshot, expect_fragments(updates))
}
}
@@ -7,6 +7,7 @@ use super::PreviousSectionState;
use super::SectionTransition;
use super::WorldStateContextFragment;
use super::WorldStateSection;
use super::WorldStateUpdate;
use codex_extension_api::ExtensionMetrics;
use codex_extension_api::RenderedWorldStateFragment;
use codex_otel::THREAD_TOOLS_FRAGMENT_BYTES_METRIC;
@@ -101,7 +102,7 @@ impl WorldStateSection for ToolsState {
PreviousSectionState::Absent | PreviousSectionState::Unknown
)
{
return (Some(current), None);
return (Some(current), Vec::new());
}
let rendered = match previous {
@@ -137,14 +138,14 @@ impl WorldStateSection for ToolsState {
record_fragment_metrics(self.metrics.as_ref(), previous, &rendered);
(
Some(current),
Some(Box::new(WorldStateContextFragment {
vec![WorldStateUpdate::fragment(WorldStateContextFragment {
fragment: RenderedWorldStateFragment::new(
"developer",
(TOOLS_OPEN_TAG, TOOLS_CLOSE_TAG),
rendered.body,
),
content_kind: ContentItemKind("tools.deferred_namespaces".to_string()),
})),
})],
)
}
}
@@ -2,7 +2,7 @@ use super::MAX_NAMESPACE_DESCRIPTION_CHARS;
use super::MAX_RENDERED_FRAGMENT_BYTES;
use super::ToolsState;
use crate::context::world_state::PreviousSectionState;
use crate::context::world_state::WorldStateSection;
use crate::context::world_state::test_support::FragmentSectionTestExt as _;
use crate::context::world_state::test_support::render_section_cases;
use codex_extension_api::ExtensionMetrics;
use codex_otel::THREAD_TOOLS_FRAGMENT_BYTES_METRIC;
@@ -68,7 +68,7 @@ fn renders_first_line_of_namespace_descriptions() {
);
let rendered = tools
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.1
.expect("tools state should render")
.render();
@@ -102,7 +102,7 @@ fn renders_added_removed_and_updated_namespace_descriptions() {
),
]);
let (snapshot, fragment) = tools.render_diff(PreviousSectionState::Known(&previous));
let (snapshot, fragment) = tools.render_fragment_diff(PreviousSectionState::Known(&previous));
let rendered = fragment.expect("tools state delta should render").render();
assert_eq!(
@@ -117,7 +117,7 @@ fn renders_added_removed_and_updated_namespace_descriptions() {
);
assert!(
tools
.render_diff(PreviousSectionState::Known(&snapshot.unwrap()))
.render_fragment_diff(PreviousSectionState::Known(&snapshot.unwrap()))
.1
.is_none()
);
@@ -139,7 +139,7 @@ fn caps_namespace_descriptions_by_character_count() {
Arc::new(RecordingMetrics::default()),
);
let (snapshot, fragment) = tools.render_diff(PreviousSectionState::Absent);
let (snapshot, fragment) = tools.render_fragment_diff(PreviousSectionState::Absent);
assert_eq!(
snapshot.unwrap(),
BTreeMap::from([
@@ -174,7 +174,7 @@ fn records_fragment_metrics_after_description_normalization_and_truncation() {
let before = format!("<tools>\nDeferred tool namespaces:\n{normalized_entries}</tools>");
let tools = ToolsState::new(namespaces, metrics.clone());
let rendered = tools
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.1
.expect("tools state should render")
.render();
@@ -203,7 +203,7 @@ fn retains_all_names_before_sharing_description_bytes() {
Arc::new(RecordingMetrics::default()),
);
let (snapshot, fragment) = tools.render_diff(PreviousSectionState::Absent);
let (snapshot, fragment) = tools.render_fragment_diff(PreviousSectionState::Absent);
let rendered = fragment.expect("tools state should render").render();
let entries = snapshot
@@ -239,7 +239,7 @@ fn retains_names_that_fit_without_reserving_an_omission_notice() {
Arc::new(RecordingMetrics::default()),
);
let rendered = tools
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.1
.expect("tools state should render")
.render();
@@ -272,7 +272,7 @@ fn preserves_raw_names_and_complete_unicode_at_the_budget_boundary() {
);
let rendered = tools
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.1
.expect("tools state should render")
.render();
@@ -300,7 +300,7 @@ fn redistributes_description_space_after_short_and_empty_descriptions() {
);
let rendered = tools
.render_diff(PreviousSectionState::Absent)
.render_fragment_diff(PreviousSectionState::Absent)
.1
.expect("tools state should render")
.render();
@@ -324,7 +324,7 @@ fn reserves_names_and_shares_descriptions_across_added_and_removed_groups() {
let previous = BTreeMap::from([("z".to_string(), "ZZZZZZZZ".to_string())]);
let rendered = tools
.render_diff(PreviousSectionState::Known(&previous))
.render_fragment_diff(PreviousSectionState::Known(&previous))
.1
.expect("tools state delta should render")
.render();
@@ -361,7 +361,7 @@ fn omits_whole_names_per_group_only_after_dropping_every_description() {
]);
let rendered = tools
.render_diff(PreviousSectionState::Known(&previous))
.render_fragment_diff(PreviousSectionState::Known(&previous))
.1
.expect("tools state delta should render")
.render();
@@ -384,7 +384,7 @@ fn reserves_the_empty_state_notice_when_all_namespaces_are_removed() {
]);
let rendered = ToolsState::new([], Arc::new(RecordingMetrics::default()))
.render_diff(PreviousSectionState::Known(&previous))
.render_fragment_diff(PreviousSectionState::Known(&previous))
.1
.expect("final tools state delta should render")
.render();
@@ -29,7 +29,10 @@ impl WorldStateSection for TestSection {
PreviousSectionState::Unknown => Some(Box::new(TestFragment("unknown".to_string()))),
PreviousSectionState::Absent | PreviousSectionState::Known(_) => None,
};
(Some(current), fragment)
(
Some(current),
WorldStateUpdate::optional_boxed_fragment(fragment),
)
}
}
@@ -75,7 +78,7 @@ impl WorldStateSection for DuplicateTestSection {
&self,
_previous: PreviousSectionState<'_, Self::Snapshot>,
) -> SectionTransition<Self::Snapshot> {
(Some(()), None)
(Some(()), Vec::new())
}
}
@@ -110,7 +113,7 @@ fn render_diff_restores_the_typed_section_snapshot() {
});
let rendered = current
.render_history_diff(Some(&previous.render_full().0), &[])
.render_history_fragment_diff(Some(&previous.render_full().0), &[])
.1;
assert_eq!(
@@ -151,7 +154,9 @@ fn extension_owned_section_uses_its_snapshot_and_renderer() {
sections: BTreeMap::from([("extension_test".to_string(), json!({"value": "before"}))]),
};
let rendered = world_state.render_history_diff(Some(&previous), &[]).1;
let rendered = world_state
.render_history_fragment_diff(Some(&previous), &[])
.1;
assert_eq!(
serde_json::to_value(world_state.render_full().0).expect("serialize world-state snapshot"),
@@ -180,7 +185,7 @@ fn extension_owned_section_uses_its_stable_id_as_content_kind_feature() {
}));
let rendered = world_state
.render_history_diff(Some(&WorldStateSnapshot::default()), &[])
.render_history_fragment_diff(Some(&WorldStateSnapshot::default()), &[])
.1;
assert_eq!(
@@ -234,7 +239,7 @@ fn missing_retained_fragment_is_rendered_again() {
assert_eq!(
world_state
.render_history_diff(Some(&previous), &[])
.render_history_fragment_diff(Some(&previous), &[])
.1
.into_iter()
.map(|fragment| fragment.body())
@@ -243,7 +248,7 @@ fn missing_retained_fragment_is_rendered_again() {
);
assert!(
world_state
.render_history_diff(Some(&previous), &[retained])
.render_history_fragment_diff(Some(&previous), &[retained])
.1
.is_empty()
);
@@ -258,7 +263,7 @@ fn extension_snapshot_updates_preserve_skipped_state() {
removed.add_extension_section(WorldStateSectionContribution::new("extension_test", |_| {
(Some(Value::Null), None)
}));
let (snapshot, fragments) = removed.render_history_diff(Some(&previous), &[]);
let (snapshot, fragments) = removed.render_history_fragment_diff(Some(&previous), &[]);
assert!(fragments.is_empty());
assert_eq!(snapshot, WorldStateSnapshot::default());
assert_eq!(
@@ -274,7 +279,7 @@ fn extension_snapshot_updates_preserve_skipped_state() {
})
.with_retained_fragment_matcher(|_, _| false),
);
let (snapshot, fragments) = invalidated.render_history_diff(Some(&previous), &[]);
let (snapshot, fragments) = invalidated.render_history_fragment_diff(Some(&previous), &[]);
assert!(fragments.is_empty());
assert_eq!(snapshot, previous);
}
@@ -291,7 +296,7 @@ fn unreadable_section_snapshot_is_treated_as_unknown() {
sections: BTreeMap::from([("test".to_string(), json!({"invalid": true}))]),
};
let rendered = current.render_history_diff(Some(&previous), &[]).1;
let rendered = current.render_history_fragment_diff(Some(&previous), &[]).1;
assert_eq!(
vec!["unknown"],
@@ -347,3 +352,110 @@ fn snapshot_merge_patch_changes_and_removes_nested_values() {
assert_eq!(previous, current);
assert_eq!(current.merge_patch_from(&current), None);
}
#[derive(Clone)]
struct MixedOutputSection(u64);
impl WorldStateSection for MixedOutputSection {
const ID: &'static str = "mixed_output";
type Snapshot = u64;
fn render_diff(
&self,
previous: PreviousSectionState<'_, Self::Snapshot>,
) -> SectionTransition<Self::Snapshot> {
if matches!(previous, PreviousSectionState::Known(previous) if *previous == self.0) {
return (None, Vec::new());
}
let updates = vec![
WorldStateUpdate::fragment(TestFragment("before tools".to_string())),
WorldStateUpdate {
placement: Placement::Prefix,
content: WorldStateUpdateContent::Item(Box::new(ResponseItem::AdditionalTools {
id: None,
role: "developer".to_string(),
tools: vec![
json!({"type": "function", "name": "example", "description": self.0.to_string()}),
],
})),
},
WorldStateUpdate {
placement: Placement::Prefix,
..WorldStateUpdate::fragment(TestFragment("prefix instructions".to_string()))
},
WorldStateUpdate {
placement: Placement::Standalone,
content: WorldStateUpdateContent::Item(Box::new(ResponseItem::AdditionalTools {
id: None,
role: "developer".to_string(),
tools: vec![json!({"type": "function", "name": "context_tool"})],
})),
},
WorldStateUpdate::fragment(TestFragment("after tools".to_string())),
];
(Some(self.0), updates)
}
}
#[test]
fn mixed_output_preserves_order_and_restores_its_snapshot() {
use crate::context_manager::updates::merge_world_state_updates;
let mut before = WorldState::default();
before.add_section(MixedOutputSection(1));
let (previous, _) = before.render_full();
let mut current = WorldState::default();
current.add_section(MixedOutputSection(2));
let (snapshot, updates) = current.render_history_diff(Some(&previous), &[]);
let expected = vec![
ContextualUserFragment::into(TestFragment("before tools".to_string())),
ResponseItem::AdditionalTools {
id: None,
role: "developer".to_string(),
tools: vec![json!({"type": "function", "name": "example", "description": "2"})],
},
ContextualUserFragment::into(TestFragment("prefix instructions".to_string())),
ResponseItem::AdditionalTools {
id: None,
role: "developer".to_string(),
tools: vec![json!({"type": "function", "name": "context_tool"})],
},
ContextualUserFragment::into(TestFragment("after tools".to_string())),
];
assert_eq!(merge_world_state_updates(updates), expected);
assert_eq!(
serde_json::to_value(&snapshot).unwrap(),
json!({"mixed_output": 2})
);
let mut restored = previous;
restored.apply_merge_patch(&snapshot.merge_patch_from(&restored).unwrap());
assert_eq!(restored, snapshot);
let (unchanged, updates) = current.render_history_diff(Some(&restored), &[]);
assert_eq!(unchanged, snapshot);
assert!(updates.is_empty());
let (full_snapshot, updates) = current.render_full();
assert_eq!(full_snapshot, snapshot);
assert_eq!(
updates
.iter()
.map(|update| matches!(update.placement, Placement::Prefix))
.collect::<Vec<_>>(),
vec![false, true, true, false, false],
);
let (prefix, context) = split_prefix_updates(updates);
assert_eq!(prefix, vec![expected[1].clone(), expected[2].clone()]);
assert_eq!(
context
.into_iter()
.map(|update| match update.content {
WorldStateUpdateContent::Fragment(fragment) => fragment.into_boxed_response_item(),
WorldStateUpdateContent::Item(item) => *item,
})
.collect::<Vec<_>>(),
vec![
expected[0].clone(),
expected[3].clone(),
expected[4].clone()
],
);
}
+3 -2
View File
@@ -18,6 +18,7 @@ use crate::context::is_guardian_context_message;
use crate::context::world_state::PersistentModeState;
use crate::context::world_state::WorldState;
use crate::context::world_state::WorldStateSnapshot;
use crate::context::world_state::WorldStateUpdate;
use crate::context_manager::normalize;
use crate::event_mapping::has_non_contextual_dev_message_content;
use crate::event_mapping::is_contextual_dev_message_content;
@@ -446,7 +447,7 @@ impl ContextManager {
pub(crate) fn update_world_state(
&mut self,
world_state: &WorldState,
) -> (Vec<Box<dyn ContextualUserFragment>>, Option<WorldStateItem>) {
) -> (Vec<WorldStateUpdate>, Option<WorldStateItem>) {
let (snapshot, fragments) =
world_state.render_history_diff(self.world_state_baseline.as_ref(), self.raw_items());
let rollout_item = self.world_state_item(&snapshot);
@@ -459,7 +460,7 @@ impl ContextManager {
world_state: &WorldState,
) -> (
WorldStateSnapshot,
Vec<Box<dyn ContextualUserFragment>>,
Vec<WorldStateUpdate>,
Option<WorldStateItem>,
) {
let (snapshot, fragments) =
@@ -4,6 +4,7 @@ use crate::context::UserInstructions;
use crate::context::world_state::SectionTransition;
use crate::context::world_state::WorldState;
use crate::context::world_state::WorldStateSection;
use crate::context::world_state::WorldStateUpdateContent;
use base64::Engine;
use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
use codex_history::CodexHarnessMetadata;
@@ -536,7 +537,7 @@ impl WorldStateSection for TestWorldStateSection {
let current = true;
let text = match previous {
crate::context::world_state::PreviousSectionState::Known(true) => {
return (None, None);
return (None, Vec::new());
}
crate::context::world_state::PreviousSectionState::Unknown => "unknown",
crate::context::world_state::PreviousSectionState::Absent
@@ -544,11 +545,10 @@ impl WorldStateSection for TestWorldStateSection {
};
(
Some(current),
Some(Box::new(UserInstructions {
vec![WorldStateUpdate::fragment(UserInstructions {
directory: None,
text: text.to_string(),
})
as Box<dyn crate::context::ContextualUserFragment>),
})],
)
}
}
@@ -602,7 +602,10 @@ fn world_state_transitions_persist_changed_state_and_skip_unchanged_state() {
assert!(full.full);
let (snapshot, fragments, patch) = history.render_step_world_state(&world_state("after"));
assert_eq!(fragments[0].body(), "after");
let WorldStateUpdateContent::Fragment(fragment) = &fragments[0].content else {
panic!("expected a text fragment");
};
assert_eq!(fragment.body(), "after");
let patch = patch.expect("changed snapshot must be persisted");
assert!(!patch.full);
let mut replayed = WorldStateSnapshot::from(&full.state);
@@ -632,7 +635,11 @@ fn world_state_reconciles_matching_legacy_history_once() {
vec!["\n\n<INSTRUCTIONS>\nunknown\n"],
fragments
.into_iter()
.map(|fragment| fragment.body())
.map(|update| match update.content {
WorldStateUpdateContent::Fragment(fragment) => fragment.body(),
WorldStateUpdateContent::Item(item) =>
panic!("expected a text fragment, got {item:?}"),
})
.collect::<Vec<_>>()
);
assert!(rollout_item.is_some_and(|item| item.full));
+33 -31
View File
@@ -1,14 +1,12 @@
use crate::context::ContextualUserFragment;
//! Groups contextual fragments without moving them across explicit response items.
use crate::context::world_state::Placement;
use crate::context::world_state::WorldStateUpdate;
use crate::context::world_state::WorldStateUpdateContent;
use codex_context_fragments::RenderedFragment;
use codex_protocol::models::InternalChatMessageMetadataPassthrough;
use codex_protocol::models::ResponseItem;
#[derive(Clone, Copy, PartialEq, Eq)]
enum MessageGroup {
Standalone,
Mergeable,
}
pub(crate) fn build_rendered_message(fragments: Vec<RenderedFragment>) -> Option<ResponseItem> {
let role = fragments.first()?.role();
debug_assert!(fragments.iter().all(|fragment| fragment.role() == role));
@@ -29,32 +27,36 @@ pub(crate) fn build_rendered_message(fragments: Vec<RenderedFragment>) -> Option
})
}
pub(crate) fn merge_contextual_fragments(
fragments: Vec<Box<dyn ContextualUserFragment>>,
) -> Vec<ResponseItem> {
let mut messages: Vec<(&str, MessageGroup, Vec<RenderedFragment>)> =
Vec::with_capacity(fragments.len());
for fragment in fragments {
let group = if fragment.requires_separate_message() {
MessageGroup::Standalone
} else {
MessageGroup::Mergeable
pub(crate) fn merge_world_state_updates(updates: Vec<WorldStateUpdate>) -> Vec<ResponseItem> {
let mut items = Vec::new();
let mut pending = Vec::<RenderedFragment>::new();
for update in updates {
let fragment = match update.content {
WorldStateUpdateContent::Fragment(fragment) => fragment,
WorldStateUpdateContent::Item(item) => {
items.extend(build_rendered_message(std::mem::take(&mut pending)));
items.push(*item);
continue;
}
};
let rendered = fragment.render_fragment();
let role = rendered.role();
match messages.last_mut() {
Some((previous_role, previous_group, rendered_fragments))
if *previous_role == role
&& *previous_group == MessageGroup::Mergeable
&& group == MessageGroup::Mergeable =>
{
rendered_fragments.push(rendered);
}
_ => messages.push((role, group, vec![rendered])),
if update.placement != Placement::Mergeable {
items.extend(build_rendered_message(std::mem::take(&mut pending)));
items.extend(build_rendered_message(vec![rendered]));
continue;
}
if pending
.first()
.is_some_and(|previous| previous.role() != rendered.role())
{
items.extend(build_rendered_message(std::mem::take(&mut pending)));
}
pending.push(rendered);
}
messages
.into_iter()
.filter_map(|(_, _, fragments)| build_rendered_message(fragments))
.collect()
items.extend(build_rendered_message(pending));
items
}
#[cfg(test)]
#[path = "updates_tests.rs"]
mod tests;
@@ -0,0 +1,116 @@
//! Verifies message grouping around explicit world-state response items.
use super::*;
use crate::context::ContextualUserFragment;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ContentItemKind;
use pretty_assertions::assert_eq;
use serde_json::json;
struct TestFragment {
role: &'static str,
text: &'static str,
markers: (&'static str, &'static str),
}
impl ContextualUserFragment for TestFragment {
fn role(&self) -> &'static str {
self.role
}
fn content_kind(&self) -> ContentItemKind {
ContentItemKind("test.context".to_string())
}
fn markers(&self) -> (&'static str, &'static str) {
self.markers
}
fn type_markers() -> (&'static str, &'static str) {
("", "")
}
fn body(&self) -> String {
self.text.to_string()
}
}
fn fragment(role: &'static str, text: &'static str) -> TestFragment {
TestFragment {
role,
text,
markers: ("", ""),
}
}
fn message(role: &str, texts: &[&str]) -> ResponseItem {
ResponseItem::Message {
id: None,
role: role.to_string(),
content: texts
.iter()
.map(|text| ContentItem::InputText {
text: text.to_string(),
})
.collect(),
phase: None,
internal_chat_message_metadata_passthrough: Some(InternalChatMessageMetadataPassthrough {
content_item_kinds: Some(
texts
.iter()
.map(|_| ContentItemKind("test.context".to_string()))
.collect(),
),
..Default::default()
}),
}
}
fn additional_tools() -> ResponseItem {
ResponseItem::AdditionalTools {
id: None,
role: "developer".to_string(),
tools: vec![json!({"type": "function", "name": "lookup", "parameters": {}})],
}
}
#[test]
fn updates_preserve_item_boundaries_roles_and_standalone_fragments() {
let tools = additional_tools();
let updates = vec![
WorldStateUpdate::fragment(fragment("developer", "before one")),
WorldStateUpdate::fragment(fragment("developer", "before two")),
WorldStateUpdate {
placement: Placement::Prefix,
..WorldStateUpdate::fragment(fragment("developer", "prefix"))
},
WorldStateUpdate::fragment(fragment("developer", "after prefix")),
WorldStateUpdate {
placement: Placement::Prefix,
content: WorldStateUpdateContent::Item(Box::new(tools.clone())),
},
WorldStateUpdate::fragment(fragment("developer", "after")),
WorldStateUpdate::fragment(fragment("developer", "standalone")).standalone(),
WorldStateUpdate::fragment(fragment("developer", "last developer")),
WorldStateUpdate::fragment(fragment("user", "user")),
WorldStateUpdate {
placement: Placement::Standalone,
content: WorldStateUpdateContent::Item(Box::new(tools.clone())),
},
];
assert_eq!(
merge_world_state_updates(updates),
vec![
message("developer", &["before one", "before two"]),
message("developer", &["prefix"]),
message("developer", &["after prefix"]),
tools.clone(),
message("developer", &["after"]),
message("developer", &["standalone"]),
message("developer", &["last developer"]),
message("user", &["user"]),
tools,
]
);
}
+115 -82
View File
@@ -30,8 +30,11 @@ use crate::context::ModelSwitchInstructions;
use crate::context::MultiAgentRoleInstructions;
use crate::context::NetworkRuleSaved;
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::WorldStateUpdateContent;
use crate::context::world_state::split_prefix_updates;
use crate::current_time::TimeProvider;
use crate::environment_selection::TurnEnvironmentSnapshot;
use crate::exec_policy::BANNED_PREFIX_SUGGESTIONS;
@@ -3599,7 +3602,7 @@ impl Session {
.await
.history
.render_step_world_state(world_state.as_ref());
let items = crate::context_manager::updates::merge_contextual_fragments(fragments);
let items = crate::context_manager::updates::merge_world_state_updates(fragments);
if !items.is_empty() {
self.record_conversation_items(turn_context, &step_context.settings.model_info, &items)
.await;
@@ -4347,92 +4350,118 @@ impl Session {
.render_fragment(),
);
}
// Render the active mode after the usage hint so it can override that hint.
let mut initial_multi_agent_mode = None;
let mut managed_developer_instructions = None;
let (world_state_snapshot, fragments) = world_state.render_full();
for fragment in fragments {
match fragment.role() {
"developer"
if fragment.markers().0 == ModelSwitchInstructions::type_markers().0 =>
{
// New-model instructions must precede the rest of the developer context.
developer_sections.insert(0, fragment.render_fragment());
let (world_state_snapshot, updates) = world_state.render_full();
let (mut items, context) = split_prefix_updates(updates);
let mut context = context.into_iter();
// Keep the initial-context grouping within each fragment run, but never across an item.
loop {
let mut next_item = None;
// Render the active mode after the usage hint so it can override that hint.
let mut initial_multi_agent_mode = None;
let mut managed_developer_instructions = None;
for update in context.by_ref() {
let fragment = match update.content {
WorldStateUpdateContent::Fragment(fragment) => fragment,
WorldStateUpdateContent::Item(item) => {
next_item = Some(*item);
break;
}
};
match fragment.role() {
"developer"
if fragment.markers().0 == ModelSwitchInstructions::type_markers().0 =>
{
// New-model instructions must precede the rest of the developer context.
developer_sections.insert(0, fragment.render_fragment());
}
"developer" if fragment.markers().0 == MULTI_AGENT_MODE_OPEN_TAG => {
initial_multi_agent_mode = Some(fragment);
}
"developer"
if fragment.markers().0
== ManagedDeveloperInstructions::type_markers().0 =>
{
managed_developer_instructions = Some(fragment);
}
"developer"
if fragment.markers().0 == MultiAgentRoleInstructions::type_markers().0 =>
{
separate_developer_sections.push(fragment.render_fragment());
}
"developer"
if update.placement == Placement::Standalone
&& fragment.markers().0.is_empty() =>
{
separate_developer_sections.push(fragment.render_fragment());
}
"developer" => developer_sections.push(fragment.render_fragment()),
"user" => contextual_user_sections.push(fragment.render_fragment()),
_ => {}
}
"developer" if fragment.markers().0 == MULTI_AGENT_MODE_OPEN_TAG => {
initial_multi_agent_mode = Some(fragment);
}
"developer"
if fragment.markers().0 == ManagedDeveloperInstructions::type_markers().0 =>
{
managed_developer_instructions = Some(fragment);
}
"developer"
if fragment.markers().0 == MultiAgentRoleInstructions::type_markers().0 =>
{
separate_developer_sections.push(fragment.render_fragment());
}
"developer"
if fragment.requires_separate_message() && fragment.markers().0.is_empty() =>
{
separate_developer_sections.push(fragment.render_fragment());
}
"developer" => developer_sections.push(fragment.render_fragment()),
"user" => contextual_user_sections.push(fragment.render_fragment()),
_ => {}
}
}
if let Some(recommended_plugins) = recommended_plugin_candidates
.as_deref()
.and_then(RecommendedPluginsInstructions::from_plugins)
{
developer_sections.push(recommended_plugins.render_fragment());
}
let mut items = Vec::with_capacity(4);
if let Some(developer_message) =
crate::context_manager::updates::build_rendered_message(developer_sections)
{
items.push(developer_message);
}
for section in separate_developer_sections {
if let Some(developer_message) =
crate::context_manager::updates::build_rendered_message(vec![section])
if next_item.is_none()
&& let Some(recommended_plugins) = recommended_plugin_candidates
.as_deref()
.and_then(RecommendedPluginsInstructions::from_plugins)
{
developer_sections.push(recommended_plugins.render_fragment());
}
if let Some(developer_message) = crate::context_manager::updates::build_rendered_message(
std::mem::take(&mut developer_sections),
) {
items.push(developer_message);
}
}
if let Some(initial_multi_agent_mode) = initial_multi_agent_mode
&& let Some(message) = crate::context_manager::updates::build_rendered_message(vec![
initial_multi_agent_mode.render_fragment(),
])
{
items.push(message);
}
if let Some(contextual_user_message) =
crate::context_manager::updates::build_rendered_message(contextual_user_sections)
{
items.push(contextual_user_message);
}
// Emit the guardian policy prompt as a separate developer item so the guardian
// subagent sees a distinct, easy-to-audit instruction block.
if separate_guardian_developer_message
&& let Some(developer_instructions) = turn_context.developer_instructions.as_deref()
&& !developer_instructions.is_empty()
&& let Some(guardian_developer_message) =
crate::context_manager::updates::build_rendered_message(vec![
GuardianPolicy::new(developer_instructions).render_fragment(),
])
{
items.push(guardian_developer_message);
}
if let Some(managed_developer_instructions) = managed_developer_instructions
&& let Some(message) = crate::context_manager::updates::build_rendered_message(vec![
managed_developer_instructions.render_fragment(),
])
{
items.push(message);
for section in separate_developer_sections.drain(..) {
if let Some(developer_message) =
crate::context_manager::updates::build_rendered_message(vec![section])
{
items.push(developer_message);
}
}
if let Some(initial_multi_agent_mode) = initial_multi_agent_mode
&& let Some(message) =
crate::context_manager::updates::build_rendered_message(vec![
initial_multi_agent_mode.render_fragment(),
])
{
items.push(message);
}
if let Some(contextual_user_message) =
crate::context_manager::updates::build_rendered_message(std::mem::take(
&mut contextual_user_sections,
))
{
items.push(contextual_user_message);
}
// Emit the guardian policy prompt as a separate developer item so the guardian
// subagent sees a distinct, easy-to-audit instruction block.
if next_item.is_none()
&& separate_guardian_developer_message
&& let Some(developer_instructions) = turn_context.developer_instructions.as_deref()
&& !developer_instructions.is_empty()
&& let Some(guardian_developer_message) =
crate::context_manager::updates::build_rendered_message(vec![
GuardianPolicy::new(developer_instructions).render_fragment(),
])
{
items.push(guardian_developer_message);
}
if let Some(managed_developer_instructions) = managed_developer_instructions
&& let Some(message) =
crate::context_manager::updates::build_rendered_message(vec![
managed_developer_instructions.render_fragment(),
])
{
items.push(message);
}
if let Some(item) = next_item {
items.push(item);
} else {
break;
}
}
// New context windows and compaction install these items directly into replacement history.
for item in &mut items {
@@ -4613,7 +4642,7 @@ impl Session {
let (fragments, rollout_item) =
state.history.update_world_state(world_state.as_ref());
(
crate::context_manager::updates::merge_contextual_fragments(fragments),
crate::context_manager::updates::merge_world_state_updates(fragments),
rollout_item,
)
};
@@ -5119,3 +5148,7 @@ mod elicitation_holders_tests;
#[cfg(test)]
pub(crate) mod tests;
#[cfg(test)]
#[path = "world_state_item_tests.rs"]
mod world_state_item_tests;
@@ -368,7 +368,7 @@ async fn record_initial_history_restores_world_state_baseline(input: BaselineTur
let turn_context = Arc::new(turn_context);
let world_state = build_world_state_from_turn_context(&session, &turn_context).await;
let expected_history = world_state
.render_full()
.render_full_fragments()
.1
.into_iter()
.map(ContextualUserFragment::into_boxed_response_item)
+1 -1
View File
@@ -11156,7 +11156,7 @@ async fn record_context_updates_and_set_reference_context_item_persists_baseline
let previous_context = Arc::new(turn_context);
let world_state = build_world_state_from_turn_context(&session, &previous_context).await;
let retained_world_state = world_state
.render_full()
.render_full_fragments()
.1
.into_iter()
.map(ContextualUserFragment::into_boxed_response_item)
@@ -0,0 +1,116 @@
//! Verifies prefix placement and item boundaries in initial session context assembly.
use super::step_context::StepContext;
use super::tests::make_session_and_context;
use crate::context::BaseInstructionsFragment;
use crate::context::ContextualUserFragment;
use crate::context::DeveloperInstructions;
use crate::context::UserInstructions;
use crate::context::world_state::Placement;
use crate::context::world_state::PreviousSectionState;
use crate::context::world_state::SectionTransition;
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 codex_protocol::models::ResponseItem;
use core_test_support::responses::strip_metadata_from_items;
use pretty_assertions::assert_eq;
use serde_json::json;
use std::sync::Arc;
struct ItemSection;
impl WorldStateSection for ItemSection {
const ID: &'static str = "items";
type Snapshot = bool;
fn render_diff(
&self,
_: PreviousSectionState<'_, Self::Snapshot>,
) -> SectionTransition<Self::Snapshot> {
(
Some(true),
vec![
WorldStateUpdate {
placement: Placement::Prefix,
content: WorldStateUpdateContent::Item(Box::new(additional_tools("example"))),
},
WorldStateUpdate {
placement: Placement::Prefix,
..WorldStateUpdate::fragment(BaseInstructionsFragment(
"base instructions".to_string(),
))
},
WorldStateUpdate::fragment(DeveloperInstructions::new("before item")),
WorldStateUpdate::fragment(UserInstructions {
directory: None,
text: "user before item".to_string(),
}),
WorldStateUpdate::fragment(DeveloperInstructions::new("separate before item"))
.standalone(),
WorldStateUpdate {
placement: Placement::Standalone,
content: WorldStateUpdateContent::Item(Box::new(additional_tools("middle"))),
},
WorldStateUpdate {
placement: Placement::Mergeable,
content: WorldStateUpdateContent::Item(Box::new(additional_tools("adjacent"))),
},
WorldStateUpdate::fragment(DeveloperInstructions::new("after item")),
WorldStateUpdate::fragment(UserInstructions {
directory: None,
text: "user after item".to_string(),
}),
WorldStateUpdate {
placement: Placement::Standalone,
content: WorldStateUpdateContent::Item(Box::new(additional_tools("trailing"))),
},
],
)
}
}
fn additional_tools(name: &str) -> ResponseItem {
ResponseItem::AdditionalTools {
id: None,
role: "developer".to_string(),
tools: vec![json!({"type": "function", "name": name})],
}
}
#[tokio::test]
async fn initial_context_preserves_world_state_items_and_snapshot() {
let (session, turn_context) = make_session_and_context().await;
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
.build_initial_context_with_world_state(&step_context, &world_state)
.await;
assert_eq!(
strip_metadata_from_items(&items),
strip_metadata_from_items(&[
additional_tools("example"),
ContextualUserFragment::into(BaseInstructionsFragment("base instructions".to_string())),
ContextualUserFragment::into(DeveloperInstructions::new("before item")),
ContextualUserFragment::into(DeveloperInstructions::new("separate before item")),
ContextualUserFragment::into(UserInstructions {
directory: None,
text: "user before item".to_string(),
}),
additional_tools("middle"),
additional_tools("adjacent"),
ContextualUserFragment::into(DeveloperInstructions::new("after item")),
ContextualUserFragment::into(UserInstructions {
directory: None,
text: "user after item".to_string(),
}),
additional_tools("trailing"),
]),
);
assert_eq!(
serde_json::to_value(snapshot).unwrap(),
json!({"items": true})
);
}
@@ -29,10 +29,6 @@ impl ContextualUserFragment for TrustedSkills {
"developer"
}
fn requires_separate_message(&self) -> bool {
true
}
fn markers(&self) -> (&'static str, &'static str) {
Self::type_markers()
}
@@ -37,10 +37,6 @@ impl ContextualUserFragment for TrustedTool {
ContentItemKind("guardian.trusted_tool".to_owned())
}
fn requires_separate_message(&self) -> bool {
true
}
fn markers(&self) -> (&'static str, &'static str) {
Self::type_markers()
}
@@ -41,10 +41,6 @@ impl ContextualUserFragment for MultiAgentRoleInstructions {
"developer"
}
fn requires_separate_message(&self) -> bool {
true
}
fn markers(&self) -> (&'static str, &'static str) {
match self {
Self::Composed { marked: true, .. } => Self::type_markers(),