mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] impl delivery_mode: current time reminders on response boundaries (#30033)
## Summary - track user-like input and tool-output boundaries in current-time reminder state - gate reminder injection when delivery_mode is after_user_or_tool_output - preserve interval debounce and forced reminders after context-window changes ## Why Training can request reminders only after user or tool-output items while keeping the existing canonical pre-inference history-injection path. ## Validation - just test -p codex-core current_time_reminders_can_follow_only_user_or_tool_outputs - just test -p codex-core current_time_reminders_follow_time_interval_and_persist_in_history - just test -p codex-core current_time_reminder_is_refreshed_after_compaction - just fix -p codex-core
This commit is contained in:
committed by
GitHub
Unverified
parent
964b138c3d
commit
adccb464d0
@@ -2775,6 +2775,7 @@ impl Session {
|
||||
let items = items.as_ref();
|
||||
{
|
||||
let mut state = self.state.lock().await;
|
||||
state.current_time_reminder.note_recorded_items(items);
|
||||
state.record_items(
|
||||
items.iter(),
|
||||
turn_context.model_info.truncation_policy.into(),
|
||||
@@ -2877,6 +2878,7 @@ impl Session {
|
||||
let response_item = items[0].clone();
|
||||
{
|
||||
let mut state = self.state.lock().await;
|
||||
state.current_time_reminder.note_recorded_items(items);
|
||||
state.record_items(
|
||||
items.iter(),
|
||||
turn_context.model_info.truncation_policy.into(),
|
||||
|
||||
@@ -1,26 +1,56 @@
|
||||
use chrono::DateTime;
|
||||
use chrono::Utc;
|
||||
use codex_features::CurrentTimeReminderDeliveryMode;
|
||||
use codex_protocol::error::CodexErr;
|
||||
use codex_protocol::error::Result as CodexResult;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
|
||||
use super::session::Session;
|
||||
use super::turn_context::TurnContext;
|
||||
use crate::context::ContextualUserFragment;
|
||||
use crate::context_manager::is_user_turn_boundary;
|
||||
|
||||
#[derive(Default)]
|
||||
pub(crate) struct CurrentTimeReminderState {
|
||||
last_delivery_time: Option<DateTime<Utc>>,
|
||||
last_window_id: Option<String>,
|
||||
pending_user_or_tool_output_boundary: bool,
|
||||
}
|
||||
|
||||
impl CurrentTimeReminderState {
|
||||
pub(super) fn note_recorded_items(&mut self, items: &[ResponseItem]) {
|
||||
if items.iter().any(|item| {
|
||||
is_user_turn_boundary(item)
|
||||
|| matches!(
|
||||
item,
|
||||
ResponseItem::FunctionCallOutput { .. }
|
||||
| ResponseItem::CustomToolCallOutput { .. }
|
||||
| ResponseItem::ToolSearchOutput { .. }
|
||||
)
|
||||
}) {
|
||||
self.pending_user_or_tool_output_boundary = true;
|
||||
}
|
||||
}
|
||||
|
||||
fn take_reminder_due(
|
||||
&mut self,
|
||||
window_id: &str,
|
||||
current_time: DateTime<Utc>,
|
||||
interval_seconds: u64,
|
||||
delivery_mode: CurrentTimeReminderDeliveryMode,
|
||||
) -> bool {
|
||||
let reminder_is_due = self.last_window_id.as_deref() != Some(window_id)
|
||||
let is_new_window = self.last_window_id.as_deref() != Some(window_id);
|
||||
// Consume the boundary for this inference even if the interval suppresses delivery.
|
||||
let follows_user_or_tool_output =
|
||||
std::mem::take(&mut self.pending_user_or_tool_output_boundary);
|
||||
if delivery_mode == CurrentTimeReminderDeliveryMode::AfterUserOrToolOutput
|
||||
&& !is_new_window
|
||||
&& !follows_user_or_tool_output
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
let reminder_is_due = is_new_window
|
||||
|| interval_seconds == 0
|
||||
|| self.last_delivery_time.is_none_or(|last_delivery_time| {
|
||||
current_time
|
||||
@@ -60,6 +90,7 @@ pub(super) async fn maybe_record_current_time_reminder(
|
||||
window_id,
|
||||
current_time,
|
||||
config.reminder_interval_seconds,
|
||||
config.delivery_mode,
|
||||
)
|
||||
};
|
||||
if !reminder_is_due {
|
||||
|
||||
@@ -12,6 +12,7 @@ use codex_core::SleepFuture;
|
||||
use codex_core::TimeFuture;
|
||||
use codex_core::TimeProvider;
|
||||
use codex_core::config::CurrentTimeReminderConfig;
|
||||
use codex_features::CurrentTimeReminderDeliveryMode;
|
||||
use codex_features::CurrentTimeSource;
|
||||
use codex_features::Feature;
|
||||
use codex_model_provider_info::built_in_model_providers;
|
||||
@@ -201,6 +202,69 @@ async fn zero_current_time_reminder_interval_delivers_when_time_moves_backward()
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn current_time_reminders_can_follow_only_user_or_tool_outputs() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let tool_args = json!({
|
||||
"command": "echo current time",
|
||||
"timeout_ms": 1_000,
|
||||
});
|
||||
let mut continue_response = ev_completed("resp-2");
|
||||
// Ask for another inference without recording a new user message or tool output.
|
||||
continue_response["response"]["end_turn"] = json!(false);
|
||||
let responses = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_function_call(
|
||||
"current-time-tool-call",
|
||||
"shell_command",
|
||||
&serde_json::to_string(&tool_args)?,
|
||||
),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-2"),
|
||||
ev_assistant_message("msg-2", "continue"),
|
||||
continue_response,
|
||||
]),
|
||||
sse(vec![ev_response_created("resp-3"), ev_completed("resp-3")]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
let test = test_codex()
|
||||
.with_config(|config| {
|
||||
enable_current_time_reminder(config, /*interval*/ 0, CurrentTimeSource::External);
|
||||
config
|
||||
.current_time_reminder
|
||||
.as_mut()
|
||||
.expect("current-time reminder should be configured")
|
||||
.delivery_mode = CurrentTimeReminderDeliveryMode::AfterUserOrToolOutput;
|
||||
})
|
||||
.with_external_time_provider(Arc::new(TestTimeProvider::default()))
|
||||
.build(&server)
|
||||
.await?;
|
||||
|
||||
test.submit_turn_with_permission_profile("first turn", PermissionProfile::Disabled)
|
||||
.await?;
|
||||
|
||||
let requests = responses.requests();
|
||||
assert_eq!(requests.len(), 3);
|
||||
assert_eq!(current_time_reminders(&requests[0]), vec![FIRST_REMINDER]);
|
||||
assert_eq!(
|
||||
current_time_reminders(&requests[1]),
|
||||
vec![FIRST_REMINDER, SECOND_REMINDER]
|
||||
);
|
||||
assert_eq!(
|
||||
current_time_reminders(&requests[2]),
|
||||
vec![FIRST_REMINDER, SECOND_REMINDER]
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn system_time_source_adds_current_time_reminder() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -260,6 +324,11 @@ async fn current_time_reminder_is_refreshed_after_compaction() -> Result<()> {
|
||||
/*interval*/ 3_000,
|
||||
CurrentTimeSource::External,
|
||||
);
|
||||
config
|
||||
.current_time_reminder
|
||||
.as_mut()
|
||||
.expect("current-time reminder should be configured")
|
||||
.delivery_mode = CurrentTimeReminderDeliveryMode::AfterUserOrToolOutput;
|
||||
})
|
||||
.with_external_time_provider(Arc::new(TestTimeProvider::default()))
|
||||
.build(&server)
|
||||
|
||||
Reference in New Issue
Block a user