mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[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`
This commit is contained in:
committed by
GitHub
Unverified
parent
0beb5c7f32
commit
08cb633c06
@@ -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,
|
||||
|
||||
@@ -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<RemoteCompactionV2Output> {
|
||||
|
||||
@@ -50,14 +50,6 @@ pub(crate) struct ContextManager {
|
||||
reference_context_item: Option<TurnContextItem>,
|
||||
}
|
||||
|
||||
#[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<BlockingLruCache<[u8; 20], Option
|
||||
)
|
||||
});
|
||||
|
||||
pub(crate) fn estimate_response_item_model_visible_bytes(item: &ResponseItem) -> i64 {
|
||||
fn estimate_response_item_model_visible_bytes(item: &ResponseItem) -> i64 {
|
||||
match item {
|
||||
ResponseItem::Reasoning {
|
||||
encrypted_content: Some(content),
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<TokenUsage> {
|
||||
|
||||
Reference in New Issue
Block a user