fix: warn instead of failing on oversized compact results
This commit is contained in:
@@ -88,7 +88,7 @@ pub const COMPACT_SUMMARY_TARGET_TOKENS: u64 = 2_000;
|
|||||||
/// See [`crate::CompactionConfig::summary_max_tokens`].
|
/// See [`crate::CompactionConfig::summary_max_tokens`].
|
||||||
pub const COMPACT_SUMMARY_MAX_TOKENS: u64 = 4_000;
|
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`].
|
/// See [`crate::CompactionConfig::result_context_max_tokens`].
|
||||||
pub const COMPACT_RESULT_CONTEXT_MAX_TOKENS: u64 = 60_000;
|
pub const COMPACT_RESULT_CONTEXT_MAX_TOKENS: u64 = 60_000;
|
||||||
|
|
||||||
|
|||||||
@@ -965,7 +965,9 @@ pub struct CompactionConfig {
|
|||||||
)]
|
)]
|
||||||
pub auto_read_budget_tokens: u64,
|
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")]
|
#[serde(default = "default_result_context_max_tokens")]
|
||||||
pub result_context_max_tokens: u64,
|
pub result_context_max_tokens: u64,
|
||||||
|
|
||||||
|
|||||||
@@ -47,7 +47,6 @@ pub(crate) enum CompactFailureCategory {
|
|||||||
Cancelled,
|
Cancelled,
|
||||||
SummaryMissing,
|
SummaryMissing,
|
||||||
SummaryTooLarge,
|
SummaryTooLarge,
|
||||||
ResultContextTooLarge,
|
|
||||||
ActiveSegmentCommit,
|
ActiveSegmentCommit,
|
||||||
Storage,
|
Storage,
|
||||||
InternalWorker,
|
InternalWorker,
|
||||||
@@ -61,7 +60,6 @@ impl CompactFailureCategory {
|
|||||||
Self::Cancelled => "cancelled",
|
Self::Cancelled => "cancelled",
|
||||||
Self::SummaryMissing => "summary_missing",
|
Self::SummaryMissing => "summary_missing",
|
||||||
Self::SummaryTooLarge => "summary_too_large",
|
Self::SummaryTooLarge => "summary_too_large",
|
||||||
Self::ResultContextTooLarge => "result_context_too_large",
|
|
||||||
Self::ActiveSegmentCommit => "active_segment_commit",
|
Self::ActiveSegmentCommit => "active_segment_commit",
|
||||||
Self::Storage => "storage",
|
Self::Storage => "storage",
|
||||||
Self::InternalWorker => "internal_worker",
|
Self::InternalWorker => "internal_worker",
|
||||||
|
|||||||
+19
-10
@@ -6260,10 +6260,25 @@ impl<C: LlmClient + 'static, St: Store> Worker<C, St> {
|
|||||||
));
|
));
|
||||||
let result_estimate = agen::token_counter::total_tokens(&new_history, &[]);
|
let result_estimate = agen::token_counter::total_tokens(&new_history, &[]);
|
||||||
if result_context_max_tokens > 0 && result_estimate.tokens > result_context_max_tokens {
|
if result_context_max_tokens > 0 && result_estimate.tokens > result_context_max_tokens {
|
||||||
return Err(WorkerError::CompactResultContextTooLarge {
|
// This history estimate includes retained items and auto-read files, not
|
||||||
tokens: result_estimate.tokens,
|
// just the summary. Treat the configured result target as advisory;
|
||||||
max: result_context_max_tokens,
|
// 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 original_entries = self.session.history().entries();
|
||||||
let derived_sources = original_entries
|
let derived_sources = original_entries
|
||||||
@@ -8036,9 +8051,6 @@ fn compact_failure_category(error: &WorkerError) -> CompactFailureCategory {
|
|||||||
WorkerError::CompactCancelled => CompactFailureCategory::Cancelled,
|
WorkerError::CompactCancelled => CompactFailureCategory::Cancelled,
|
||||||
WorkerError::CompactSummaryMissing => CompactFailureCategory::SummaryMissing,
|
WorkerError::CompactSummaryMissing => CompactFailureCategory::SummaryMissing,
|
||||||
WorkerError::CompactSummaryTooLarge { .. } => CompactFailureCategory::SummaryTooLarge,
|
WorkerError::CompactSummaryTooLarge { .. } => CompactFailureCategory::SummaryTooLarge,
|
||||||
WorkerError::CompactResultContextTooLarge { .. } => {
|
|
||||||
CompactFailureCategory::ResultContextTooLarge
|
|
||||||
}
|
|
||||||
WorkerError::WorkerStore(_)
|
WorkerError::WorkerStore(_)
|
||||||
| WorkerError::CompactActiveSegmentChanged
|
| WorkerError::CompactActiveSegmentChanged
|
||||||
| WorkerError::SegmentActivationIncomplete { .. } => {
|
| WorkerError::SegmentActivationIncomplete { .. } => {
|
||||||
@@ -8156,9 +8168,6 @@ pub enum WorkerError {
|
|||||||
#[error("compact summary too large: {tokens} tokens exceeds max {max}")]
|
#[error("compact summary too large: {tokens} tokens exceeds max {max}")]
|
||||||
CompactSummaryTooLarge { tokens: u64, max: u64 },
|
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}")]
|
#[error("invalid system prompt template: {source}")]
|
||||||
InvalidSystemPromptTemplate {
|
InvalidSystemPromptTemplate {
|
||||||
#[source]
|
#[source]
|
||||||
|
|||||||
@@ -581,6 +581,157 @@ fn system_texts_in_sink_session_start(
|
|||||||
Vec::new()
|
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::<Event>(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::<Event>(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]
|
#[tokio::test]
|
||||||
async fn active_segment_cas_rejects_stale_compaction_writer() {
|
async fn active_segment_cas_rejects_stale_compaction_writer() {
|
||||||
let client = MockClient::new(vec![
|
let client = MockClient::new(vec![
|
||||||
|
|||||||
@@ -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.
|
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 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.
|
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.
|
||||||
|
|||||||
Reference in New Issue
Block a user