From 08cb633c06a27d25872d0132fbd9c749556d7653 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Mon, 8 Jun 2026 19:23:35 -0700 Subject: [PATCH] [codex] Remove remote compaction failure log (#27106) ## Why `log_remote_compact_failure` was the only consumer of the compact-request logging payload and most of the token-usage breakdown fields. Once that failure log is removed, keeping the surrounding carrier types leaves dead plumbing in the compaction path and context manager. ## What changed - Remove `log_remote_compact_failure`, `CompactRequestLogData`, and the v2 wrapper that only fed that log. - Let both remote compaction implementations return the original compaction error directly. - Replace `TotalTokenUsageBreakdown` with a narrow helper that returns only the remaining value needed by compaction analytics. - Keep `estimate_response_item_model_visible_bytes` private to the context manager implementation. ## Validation - `cargo check -p codex-core` --- codex-rs/core/src/compact_remote.rs | 62 +------------------- codex-rs/core/src/compact_remote_v2.rs | 37 ++---------- codex-rs/core/src/context_manager/history.rs | 41 ++----------- codex-rs/core/src/context_manager/mod.rs | 2 - codex-rs/core/src/session/mod.rs | 7 ++- 5 files changed, 17 insertions(+), 132 deletions(-) diff --git a/codex-rs/core/src/compact_remote.rs b/codex-rs/core/src/compact_remote.rs index 5fca191b8..bd852b4e7 100644 --- a/codex-rs/core/src/compact_remote.rs +++ b/codex-rs/core/src/compact_remote.rs @@ -8,8 +8,6 @@ use crate::compact::InitialContextInjection; use crate::compact::compaction_status_from_result; use crate::compact::insert_initial_context_before_last_real_user_or_summary; use crate::context_manager::ContextManager; -use crate::context_manager::TotalTokenUsageBreakdown; -use crate::context_manager::estimate_response_item_model_visible_bytes; use crate::hook_runtime::PostCompactHookOutcome; use crate::hook_runtime::PreCompactHookOutcome; use crate::hook_runtime::run_post_compact_hooks; @@ -35,9 +33,7 @@ use codex_protocol::protocol::CompactedItem; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::TurnStartedEvent; use codex_rollout_trace::CompactionCheckpointTracePayload; -use futures::TryFutureExt; use tokio_util::sync::CancellationToken; -use tracing::error; use tracing::info; const CONTEXT_WINDOW_TRUNCATED_OUTPUT_MESSAGE: &str = @@ -199,9 +195,8 @@ async fn run_remote_compact_task_inner_impl( } if estimated_deleted_tokens > 0 { let max_local_deleted_tokens = sess - .get_total_token_usage_breakdown() - .await - .estimated_tokens_of_items_added_since_last_successful_api_response; + .estimated_tokens_after_last_model_generated_item() + .await; analytics_details.active_context_tokens_before = analytics_details .active_context_tokens_before .map(|active_context_tokens_before| { @@ -252,18 +247,6 @@ async fn run_remote_compact_task_inner_impl( &compaction_trace, turn_metadata_header.as_deref(), ) - .or_else(|err| async { - let total_usage_breakdown = sess.get_total_token_usage_breakdown().await; - let compact_request_log_data = - build_compact_request_log_data(&prompt.input, &prompt.base_instructions.text); - log_remote_compact_failure( - turn_context, - &compact_request_log_data, - total_usage_breakdown, - &err, - ); - Err(err) - }) .await?; new_history = process_compacted_history( sess.as_ref(), @@ -363,47 +346,6 @@ pub(crate) fn should_keep_compacted_history_item(item: &ResponseItem) -> bool { } } -#[derive(Debug)] -pub(crate) struct CompactRequestLogData { - failing_compaction_request_model_visible_bytes: i64, -} - -pub(crate) fn build_compact_request_log_data( - input: &[ResponseItem], - instructions: &str, -) -> CompactRequestLogData { - let failing_compaction_request_model_visible_bytes = input - .iter() - .map(estimate_response_item_model_visible_bytes) - .fold( - i64::try_from(instructions.len()).unwrap_or(i64::MAX), - i64::saturating_add, - ); - - CompactRequestLogData { - failing_compaction_request_model_visible_bytes, - } -} - -pub(crate) fn log_remote_compact_failure( - turn_context: &TurnContext, - log_data: &CompactRequestLogData, - total_usage_breakdown: TotalTokenUsageBreakdown, - err: &CodexErr, -) { - error!( - turn_id = %turn_context.sub_id, - last_api_response_total_tokens = total_usage_breakdown.last_api_response_total_tokens, - all_history_items_model_visible_bytes = total_usage_breakdown.all_history_items_model_visible_bytes, - estimated_tokens_of_items_added_since_last_successful_api_response = total_usage_breakdown.estimated_tokens_of_items_added_since_last_successful_api_response, - estimated_bytes_of_items_added_since_last_successful_api_response = total_usage_breakdown.estimated_bytes_of_items_added_since_last_successful_api_response, - model_context_window_tokens = ?turn_context.model_context_window(), - failing_compaction_request_model_visible_bytes = log_data.failing_compaction_request_model_visible_bytes, - compact_error = %err, - "remote compaction failed" - ); -} - pub(crate) fn trim_function_call_history_to_fit_context_window( history: &mut ContextManager, turn_context: &TurnContext, diff --git a/codex-rs/core/src/compact_remote_v2.rs b/codex-rs/core/src/compact_remote_v2.rs index 1d4eb8ad5..a80d4dd56 100644 --- a/codex-rs/core/src/compact_remote_v2.rs +++ b/codex-rs/core/src/compact_remote_v2.rs @@ -8,8 +8,6 @@ use crate::compact::CompactionAnalyticsAttempt; use crate::compact::CompactionAnalyticsDetails; use crate::compact::InitialContextInjection; use crate::compact::compaction_status_from_result; -use crate::compact_remote::build_compact_request_log_data; -use crate::compact_remote::log_remote_compact_failure; use crate::compact_remote::process_compacted_history; use crate::compact_remote::should_keep_compacted_history_item; use crate::compact_remote::trim_function_call_history_to_fit_context_window; @@ -212,9 +210,8 @@ async fn run_remote_compact_task_inner_impl( } if estimated_deleted_tokens > 0 { let max_local_deleted_tokens = sess - .get_total_token_usage_breakdown() - .await - .estimated_tokens_of_items_added_since_last_successful_api_response; + .estimated_tokens_after_last_model_generated_item() + .await; analytics_details.active_context_tokens_before = analytics_details .active_context_tokens_before .map(|active_context_tokens_before| { @@ -354,12 +351,9 @@ async fn run_remote_compaction_request_v2( match result { Ok(compaction_output) => return Ok(compaction_output), - Err(err) if !err.is_retryable() => { - log_remote_compaction_request_failure(sess, turn_context, prompt, &err).await; - return Err(err); - } + Err(err) if !err.is_retryable() => return Err(err), Err(err) => { - if let Err(err) = handle_retryable_response_stream_error( + handle_retryable_response_stream_error( &mut retries, max_retries, err, @@ -368,33 +362,12 @@ async fn run_remote_compaction_request_v2( turn_context, ResponsesStreamRequest::RemoteCompactionV2, ) - .await - { - log_remote_compaction_request_failure(sess, turn_context, prompt, &err).await; - return Err(err); - } + .await?; } } } } -async fn log_remote_compaction_request_failure( - sess: &Session, - turn_context: &TurnContext, - prompt: &Prompt, - err: &CodexErr, -) { - let total_usage_breakdown = sess.get_total_token_usage_breakdown().await; - let compact_request_log_data = - build_compact_request_log_data(&prompt.input, &prompt.base_instructions.text); - log_remote_compact_failure( - turn_context, - &compact_request_log_data, - total_usage_breakdown, - err, - ); -} - async fn collect_compaction_output( mut stream: ResponseStream, ) -> CodexResult { diff --git a/codex-rs/core/src/context_manager/history.rs b/codex-rs/core/src/context_manager/history.rs index b8bcf9c84..479ccc89c 100644 --- a/codex-rs/core/src/context_manager/history.rs +++ b/codex-rs/core/src/context_manager/history.rs @@ -50,14 +50,6 @@ pub(crate) struct ContextManager { reference_context_item: Option, } -#[derive(Debug, Clone, Copy, Default)] -pub(crate) struct TotalTokenUsageBreakdown { - pub last_api_response_total_tokens: i64, - pub all_history_items_model_visible_bytes: i64, - pub estimated_tokens_of_items_added_since_last_successful_api_response: i64, - pub estimated_bytes_of_items_added_since_last_successful_api_response: i64, -} - impl ContextManager { pub(crate) fn new() -> Self { Self { @@ -321,32 +313,11 @@ impl ContextManager { } } - pub(crate) fn get_total_token_usage_breakdown(&self) -> TotalTokenUsageBreakdown { - let last_usage = self - .token_info - .as_ref() - .map(|info| info.last_token_usage.clone()) - .unwrap_or_default(); - let items_after_last_model_generated = self.items_after_last_model_generated_item(); - - TotalTokenUsageBreakdown { - last_api_response_total_tokens: last_usage.total_tokens, - all_history_items_model_visible_bytes: self - .items - .iter() - .map(estimate_response_item_model_visible_bytes) - .fold(0i64, i64::saturating_add), - estimated_tokens_of_items_added_since_last_successful_api_response: - items_after_last_model_generated - .iter() - .map(estimate_item_token_count) - .fold(0i64, i64::saturating_add), - estimated_bytes_of_items_added_since_last_successful_api_response: - items_after_last_model_generated - .iter() - .map(estimate_response_item_model_visible_bytes) - .fold(0i64, i64::saturating_add), - } + pub(crate) fn estimated_tokens_after_last_model_generated_item(&self) -> i64 { + self.items_after_last_model_generated_item() + .iter() + .map(estimate_item_token_count) + .fold(0i64, i64::saturating_add) } /// This function enforces a couple of invariants on the in-memory history: @@ -532,7 +503,7 @@ static ORIGINAL_IMAGE_ESTIMATE_CACHE: LazyLock i64 { +fn estimate_response_item_model_visible_bytes(item: &ResponseItem) -> i64 { match item { ResponseItem::Reasoning { encrypted_content: Some(content), diff --git a/codex-rs/core/src/context_manager/mod.rs b/codex-rs/core/src/context_manager/mod.rs index 2295c49df..f811c8b37 100644 --- a/codex-rs/core/src/context_manager/mod.rs +++ b/codex-rs/core/src/context_manager/mod.rs @@ -3,7 +3,5 @@ mod normalize; pub(crate) mod updates; pub(crate) use history::ContextManager; -pub(crate) use history::TotalTokenUsageBreakdown; -pub(crate) use history::estimate_response_item_model_visible_bytes; pub(crate) use history::is_user_turn_boundary; pub(crate) use history::truncate_function_output_payload; diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index f3e506d40..bb66e8684 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -182,7 +182,6 @@ use crate::config::PermissionProfileState; use crate::config::StartedNetworkProxy; use crate::config::resolve_web_search_mode_for_turn; use crate::context_manager::ContextManager; -use crate::context_manager::TotalTokenUsageBreakdown; use crate::thread_rollout_truncation::initial_history_has_prior_user_turns; use codex_config::CONFIG_TOML_FILE; use codex_config::ConfigLayerSource; @@ -1145,9 +1144,11 @@ impl Session { state.auto_compact_window_snapshot() } - pub(crate) async fn get_total_token_usage_breakdown(&self) -> TotalTokenUsageBreakdown { + pub(crate) async fn estimated_tokens_after_last_model_generated_item(&self) -> i64 { let state = self.state.lock().await; - state.history.get_total_token_usage_breakdown() + state + .history + .estimated_tokens_after_last_model_generated_item() } pub(crate) async fn total_token_usage(&self) -> Option {