From 75fc7c921b477ebb0fd5204fa9cbff3db547809d Mon Sep 17 00:00:00 2001 From: Hare Date: Sat, 3 Oct 2026 21:56:58 +0900 Subject: [PATCH] fix: warn instead of failing on oversized compact results --- crates/manifest/src/defaults.rs | 2 +- crates/manifest/src/lib.rs | 4 +- crates/worker/src/compact/telemetry.rs | 2 - crates/worker/src/worker.rs | 29 ++-- crates/worker/tests/compact_events_test.rs | 151 +++++++++++++++++++++ docs/design/compaction.md | 6 + 6 files changed, 180 insertions(+), 14 deletions(-) diff --git a/crates/manifest/src/defaults.rs b/crates/manifest/src/defaults.rs index d208d244a..bf4755b82 100644 --- a/crates/manifest/src/defaults.rs +++ b/crates/manifest/src/defaults.rs @@ -88,7 +88,7 @@ pub const COMPACT_SUMMARY_TARGET_TOKENS: u64 = 2_000; /// See [`crate::CompactionConfig::summary_max_tokens`]. pub const COMPACT_SUMMARY_MAX_TOKENS: u64 = 4_000; -/// Dry-run cap for the compacted session's initial request context. +/// Warning threshold for the assembled compacted history. Compaction continues. /// See [`crate::CompactionConfig::result_context_max_tokens`]. pub const COMPACT_RESULT_CONTEXT_MAX_TOKENS: u64 = 60_000; diff --git a/crates/manifest/src/lib.rs b/crates/manifest/src/lib.rs index cd126532d..77ee30896 100644 --- a/crates/manifest/src/lib.rs +++ b/crates/manifest/src/lib.rs @@ -965,7 +965,9 @@ pub struct CompactionConfig { )] pub auto_read_budget_tokens: u64, - /// Dry-run cap for the compacted session's initial request context. + /// Warning threshold for the assembled compacted history estimate, including + /// retained items and auto-read files. Exceeding it does not abort compaction; + /// request-time context safety checks still apply. `0` disables this warning. #[serde(default = "default_result_context_max_tokens")] pub result_context_max_tokens: u64, diff --git a/crates/worker/src/compact/telemetry.rs b/crates/worker/src/compact/telemetry.rs index eee3dafd8..2ab484d05 100644 --- a/crates/worker/src/compact/telemetry.rs +++ b/crates/worker/src/compact/telemetry.rs @@ -47,7 +47,6 @@ pub(crate) enum CompactFailureCategory { Cancelled, SummaryMissing, SummaryTooLarge, - ResultContextTooLarge, ActiveSegmentCommit, Storage, InternalWorker, @@ -61,7 +60,6 @@ impl CompactFailureCategory { Self::Cancelled => "cancelled", Self::SummaryMissing => "summary_missing", Self::SummaryTooLarge => "summary_too_large", - Self::ResultContextTooLarge => "result_context_too_large", Self::ActiveSegmentCommit => "active_segment_commit", Self::Storage => "storage", Self::InternalWorker => "internal_worker", diff --git a/crates/worker/src/worker.rs b/crates/worker/src/worker.rs index 1e0441297..98b073ac5 100644 --- a/crates/worker/src/worker.rs +++ b/crates/worker/src/worker.rs @@ -6260,10 +6260,25 @@ impl Worker { )); let result_estimate = agen::token_counter::total_tokens(&new_history, &[]); if result_context_max_tokens > 0 && result_estimate.tokens > result_context_max_tokens { - return Err(WorkerError::CompactResultContextTooLarge { - tokens: result_estimate.tokens, - max: result_context_max_tokens, - }); + // This history estimate includes retained items and auto-read files, not + // just the summary. Treat the configured result target as advisory; + // request-time context safety checks still apply after activation. + warn!( + result_tokens = result_estimate.tokens, + warning_threshold = result_context_max_tokens, + retained_tokens = retained_estimate.tokens, + summary_tokens, + auto_read_tokens, + "compacted result context exceeds warning threshold; continuing" + ); + self.alert( + AlertLevel::Warn, + AlertSource::Compactor, + format!( + "compacted result context is larger than expected (≈{} tokens; warning threshold {}); continuing compaction", + result_estimate.tokens, result_context_max_tokens + ), + ); } let original_entries = self.session.history().entries(); let derived_sources = original_entries @@ -8036,9 +8051,6 @@ fn compact_failure_category(error: &WorkerError) -> CompactFailureCategory { WorkerError::CompactCancelled => CompactFailureCategory::Cancelled, WorkerError::CompactSummaryMissing => CompactFailureCategory::SummaryMissing, WorkerError::CompactSummaryTooLarge { .. } => CompactFailureCategory::SummaryTooLarge, - WorkerError::CompactResultContextTooLarge { .. } => { - CompactFailureCategory::ResultContextTooLarge - } WorkerError::WorkerStore(_) | WorkerError::CompactActiveSegmentChanged | WorkerError::SegmentActivationIncomplete { .. } => { @@ -8156,9 +8168,6 @@ pub enum WorkerError { #[error("compact summary too large: {tokens} tokens exceeds max {max}")] CompactSummaryTooLarge { tokens: u64, max: u64 }, - #[error("compacted result context too large: {tokens} tokens exceeds max {max}")] - CompactResultContextTooLarge { tokens: u64, max: u64 }, - #[error("invalid system prompt template: {source}")] InvalidSystemPromptTemplate { #[source] diff --git a/crates/worker/tests/compact_events_test.rs b/crates/worker/tests/compact_events_test.rs index 2c71f3e46..1a1fdc3eb 100644 --- a/crates/worker/tests/compact_events_test.rs +++ b/crates/worker/tests/compact_events_test.rs @@ -581,6 +581,157 @@ fn system_texts_in_sink_session_start( Vec::new() } +#[tokio::test] +async fn result_context_warning_preserves_auto_read_and_continues() { + // The auto-read contents alone exceed the warning threshold in the middle + // case. Disabled and larger thresholds must remain silent. + for (threshold, expect_warning) in [(0, false), (1_000, true), (50_000, false)] { + let mark_input = serde_json::json!({ "file_path": "context.txt" }).to_string(); + let client = MockClient::new(vec![ + single_text_events("retained response"), + write_summary_tool_use_events("summary-1", "replacement summary"), + vec![ + LlmEvent::tool_use_start(0, "mark-1", "mark_read_required"), + LlmEvent::tool_input_delta(0, mark_input), + LlmEvent::tool_use_stop(0), + LlmEvent::Status(StatusEvent { + status: ResponseStatus::Completed, + }), + ], + single_text_events("done"), + text_events_with_usage("continued on replacement", 4_000), + ]); + let calls = Arc::clone(&client.call_count); + let manifest = MANUAL_ONLY_MANIFEST_TOML.replace( + "[compaction]", + &format!("[compaction]\nresult_context_max_tokens = {threshold}"), + ); + let (mut worker, store) = make_worker_with_manifest_and_store(&manifest, client).await; + let file_contents = "load-bearing file contents\n".repeat(300); + let file = worker + .filesystem_authority() + .as_local() + .unwrap() + .root + .join("context.txt"); + std::fs::write(file, &file_contents).unwrap(); + let (tx, mut rx) = broadcast::channel::(64); + worker.attach_alerter(worker::Alerter::new(tx)); + worker.run_text("retained input").await.unwrap(); + let source_segment = worker.segment_id(); + let _ = drain(&mut rx); + + let replacement_segment = worker.compact(100).await.unwrap(); + assert_ne!(replacement_segment, source_segment); + assert_eq!(worker.segment_id(), replacement_segment); + assert_eq!( + WorkerMetadataStore::read_by_name(&store, "test-worker") + .unwrap() + .unwrap() + .active + .unwrap() + .segment_id, + Some(replacement_segment) + ); + let system_texts = system_texts_in_sink_session_start(&worker); + assert!( + system_texts + .iter() + .any(|text| text.contains("replacement summary")) + ); + assert!( + system_texts + .iter() + .any(|text| text.contains(&file_contents)) + ); + let result_tokens = agen::token_counter::total_tokens(&worker.history(), &[]).tokens; + assert!(result_tokens > 1_000 && result_tokens < 50_000); + assert!(worker.history().iter().any(|item| { + matches!(item, Item::Message { content, .. } + if content.iter().any(|part| part.as_text().contains("retained response"))) + })); + let warnings = drain(&mut rx) + .into_iter() + .filter(|event| { + matches!(event, Event::Alert(alert) + if alert.level == protocol::AlertLevel::Warn + && alert.source == protocol::AlertSource::Compactor + && alert.message.contains("compacted result context")) + }) + .count(); + assert_eq!(warnings, usize::from(expect_warning)); + + worker.run_text("continue").await.unwrap(); + assert_eq!(calls.load(Ordering::SeqCst), 5); + assert_eq!(worker.segment_id(), replacement_segment); + let metrics = session_metrics::read_session_metrics(&store, worker.session_id()).unwrap(); + let finish = metrics + .iter() + .find(|record| record.metric.name == "compact.finish") + .unwrap(); + assert_eq!(finish.metric.dimensions["outcome"], "succeeded"); + let auto_read = metrics + .iter() + .find(|record| record.metric.name == "compact.auto_read_tokens") + .unwrap(); + assert!(auto_read.metric.value.unwrap() > 1_000.0); + } +} + +#[tokio::test] +async fn result_context_warning_does_not_stop_automatic_resume() { + let client = MockClient::new(vec![ + text_events_with_usage("seed", 100_000), + write_summary_tool_use_events("summary-1", "replacement summary"), + single_text_events("done"), + text_events_with_usage("continued on replacement", 1_000), + ]); + let calls = Arc::clone(&client.call_count); + let manifest = MID_TURN_MANIFEST_TOML.replace( + "compact_request_threshold = 100", + "compact_request_threshold = 50000\nresult_context_max_tokens = 1", + ); + let (mut worker, store) = make_worker_with_manifest_and_store(&manifest, client).await; + let (tx, mut rx) = broadcast::channel::(64); + worker.attach_alerter(worker::Alerter::new(tx)); + worker.run_text("seed input").await.unwrap(); + let source_segment = worker.segment_id(); + + worker.run_text("continue").await.unwrap(); + assert_ne!(worker.segment_id(), source_segment); + assert_eq!(calls.load(Ordering::SeqCst), 4); + assert!( + drain(&mut rx) + .iter() + .any(|event| matches!(event, Event::Alert(alert) + if alert.level == protocol::AlertLevel::Warn + && alert.source == protocol::AlertSource::Compactor + && alert.message.contains("compacted result context"))) + ); + let entries = store + .read_all(worker.session_id(), worker.segment_id()) + .unwrap(); + assert!(entries.iter().any(|entry| matches!( + entry, + LogEntry::RunResumed { + source: protocol::RunResumeSource::Compaction, + .. + } + ))); + assert!(entries.iter().any(|entry| matches!( + entry, + LogEntry::RunCompleted { + result: agen::EngineResult::Finished, + .. + } + ))); + assert!( + !entries + .iter() + .any(|entry| matches!(entry, LogEntry::RunErrored { .. })) + ); +} + #[tokio::test] async fn active_segment_cas_rejects_stale_compaction_writer() { let client = MockClient::new(vec![ diff --git a/docs/design/compaction.md b/docs/design/compaction.md index fbe6e62d8..bcd66f0c9 100644 --- a/docs/design/compaction.md +++ b/docs/design/compaction.md @@ -20,6 +20,12 @@ Compaction output is not automatically safe. The post-compact context must be re A `just_compacted` flag must not bypass safety checks. It is easy for a compact summary, retained tail, or prompt resource change to still exceed a context limit. +`result_context_max_tokens` is an advisory warning threshold for the assembled replacement +history, not a hard activation cap or a provider context limit. Its estimate includes the summary, +auto-read files, references, retained tail, and TaskStore context. Exceeding it emits a Compactor +warning and continues without discarding the generated summary or stripping auto-read/retained +content. `0` disables this warning. Normal request-time safety checks still apply after activation. + ## Large sessions Large-session compaction should not send an entire prefix transcript as the summary input. Prefer bounded overview/index inputs plus exploration, then keep the retained tail small and explicit.