refactor: move memory extraction into lifecycle feature
This commit is contained in:
@@ -578,138 +578,6 @@ async fn mid_turn_compact_success_broadcasts_start_and_done() {
|
||||
assert_eq!(new_id_in_event, Some(worker.segment_id()));
|
||||
}
|
||||
|
||||
/// Regression: `Worker::compact()` must reset the in-memory
|
||||
/// `extract_pointer` so extract keeps firing on the new compacted
|
||||
/// session.
|
||||
///
|
||||
/// Without the reset, the pointer's `processed_through_history_len`
|
||||
/// holds the old (typically large) item count, while the new compacted
|
||||
/// session starts with a much shorter history (`[summary, ...]`).
|
||||
/// `cumulative_input_tokens_since` would then filter every new
|
||||
/// usage record out (their `history_len` is below the stale pointer)
|
||||
/// and extract would never re-fire for the rest of the process.
|
||||
const EXTRACT_PLUS_COMPACT_MANIFEST: &str = r#"
|
||||
[worker]
|
||||
name = "test-worker"
|
||||
pwd = "./"
|
||||
|
||||
[model]
|
||||
scheme = "anthropic"
|
||||
model_id = "test-model"
|
||||
|
||||
[engine]
|
||||
max_tokens = 100
|
||||
|
||||
[memory]
|
||||
workspace_id = "test-workspace"
|
||||
settings_revision = 1
|
||||
language = "English"
|
||||
extract_threshold = 1
|
||||
|
||||
[compaction]
|
||||
compact_threshold = 1
|
||||
compact_retained_tokens = 0
|
||||
|
||||
[[scope.allow]]
|
||||
target = "./"
|
||||
permission = "write"
|
||||
"#;
|
||||
|
||||
fn finish_memory_extraction_tool_use_events(call_id: &str) -> Vec<LlmEvent> {
|
||||
let input = serde_json::json!({
|
||||
"staged_count": 0,
|
||||
"no_candidates_reason": "test run has no durable candidates"
|
||||
})
|
||||
.to_string();
|
||||
vec![
|
||||
LlmEvent::tool_use_start(0, call_id, "FinishMemoryExtraction"),
|
||||
LlmEvent::tool_input_delta(0, input),
|
||||
LlmEvent::tool_use_stop(0),
|
||||
LlmEvent::Status(StatusEvent {
|
||||
status: ResponseStatus::Completed,
|
||||
}),
|
||||
]
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn compact_resets_extract_pointer_so_extract_can_fire_again() {
|
||||
// Mock LLM responses, in call order:
|
||||
// [0] first run with usage(1000) so extract threshold (=1) fires.
|
||||
// [1] extract worker invokes FinishMemoryExtraction with empty output.
|
||||
// [2] extract worker closes after the tool result.
|
||||
// [3] compact worker invokes write_summary.
|
||||
// [4] compact worker closes after the tool result.
|
||||
let client = MockClient::new(vec![
|
||||
text_events_with_usage("hi", 1000),
|
||||
finish_memory_extraction_tool_use_events("ec1"),
|
||||
single_text_events("done"),
|
||||
write_summary_tool_use_events("sc1", "summary"),
|
||||
single_text_events("done"),
|
||||
]);
|
||||
let mut worker = make_worker_with_manifest(EXTRACT_PLUS_COMPACT_MANIFEST, client).await;
|
||||
|
||||
worker.run_text("first").await.unwrap();
|
||||
|
||||
// extract fires; pointer becomes Some.
|
||||
worker.try_post_run_extract().await.unwrap();
|
||||
assert!(
|
||||
worker.extract_pointer().is_some(),
|
||||
"extract_pointer should be Some after a successful extract"
|
||||
);
|
||||
|
||||
// Compact runs. Without the fix the in-memory pointer would still
|
||||
// reference the old Segment's history_len.
|
||||
worker.try_pre_run_compact().await;
|
||||
assert!(
|
||||
worker.extract_pointer().is_none(),
|
||||
"extract_pointer must be reset to None after compact (matches cold-restore on the new Segment)"
|
||||
);
|
||||
}
|
||||
|
||||
/// `extract_threshold = 0` is treated as "disabled" — without this, a
|
||||
/// raw `>=` comparison against `tokens_since` would fire extract on
|
||||
/// every post-run regardless of activity. Mirrors the consolidation
|
||||
/// zero-threshold convention so users have a single way to opt out
|
||||
/// without removing the `[memory]` section.
|
||||
const EXTRACT_THRESHOLD_ZERO_MANIFEST: &str = r#"
|
||||
[worker]
|
||||
name = "test-worker"
|
||||
pwd = "./"
|
||||
|
||||
[model]
|
||||
scheme = "anthropic"
|
||||
model_id = "test-model"
|
||||
|
||||
[engine]
|
||||
max_tokens = 100
|
||||
|
||||
[memory]
|
||||
extract_threshold = 0
|
||||
|
||||
[[scope.allow]]
|
||||
target = "./"
|
||||
permission = "write"
|
||||
"#;
|
||||
|
||||
#[tokio::test]
|
||||
async fn extract_threshold_zero_is_disabled() {
|
||||
// Mock provides exactly one response — the first run. If extract
|
||||
// were treated as "fire on any change" because of `tokens_since >= 0`,
|
||||
// it would call into the extract worker and exhaust the mock.
|
||||
let client = MockClient::new(vec![text_events_with_usage("hi", 1000)]);
|
||||
let mut worker = make_worker_with_manifest(EXTRACT_THRESHOLD_ZERO_MANIFEST, client).await;
|
||||
|
||||
worker.run_text("first").await.unwrap();
|
||||
worker
|
||||
.try_post_run_extract()
|
||||
.await
|
||||
.expect("extract_threshold=0 must skip silently, not fail");
|
||||
assert!(
|
||||
worker.extract_pointer().is_none(),
|
||||
"no extract should have run — pointer must remain None"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn pre_run_compact_failure_broadcasts_start_and_failed() {
|
||||
// Only the first run has a response. Compaction will run the
|
||||
@@ -746,112 +614,6 @@ async fn pre_run_compact_failure_broadcasts_start_and_failed() {
|
||||
);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Detached post-run memory jobs (`spawn_post_run_memory_jobs` /
|
||||
// `wait_for_memory_jobs`). Covers the detach round-trip and the structural
|
||||
// invariant that the cloned memory-task Worker shares `SegmentState` with the
|
||||
// source Worker, so that `save_extension` from the background extract does not
|
||||
// leave the next turn's `save_user_input` looking at a stale session pointer.
|
||||
|
||||
const EXTRACT_NO_COMPACT_MANIFEST: &str = r#"
|
||||
[worker]
|
||||
name = "test-worker"
|
||||
pwd = "./"
|
||||
|
||||
[model]
|
||||
scheme = "anthropic"
|
||||
model_id = "test-model"
|
||||
|
||||
[engine]
|
||||
max_tokens = 100
|
||||
|
||||
[memory]
|
||||
workspace_id = "test-workspace"
|
||||
settings_revision = 1
|
||||
language = "English"
|
||||
extract_threshold = 1
|
||||
|
||||
[[scope.allow]]
|
||||
target = "./"
|
||||
permission = "write"
|
||||
"#;
|
||||
|
||||
#[tokio::test]
|
||||
async fn extract_large_unprocessed_range_does_not_abort_on_input_occupancy() {
|
||||
let client = MockClient::new(vec![
|
||||
text_events_with_usage("recorded", 1000),
|
||||
finish_memory_extraction_tool_use_events("ec-large"),
|
||||
single_text_events("done"),
|
||||
]);
|
||||
let mut worker = make_worker_with_manifest(EXTRACT_NO_COMPACT_MANIFEST, client).await;
|
||||
|
||||
let large_request = format!("remember this large slice: {}", "x ".repeat(200_000));
|
||||
worker.run_text(&large_request).await.unwrap();
|
||||
|
||||
worker.try_post_run_extract().await.expect(
|
||||
"large unprocessed extract ranges must reach the extract worker, not abort locally",
|
||||
);
|
||||
assert!(
|
||||
worker.extract_pointer().is_some(),
|
||||
"successful extract should advance the pointer even when the input range is large"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn spawn_and_wait_drives_extract_to_completion() {
|
||||
let client = MockClient::new(vec![
|
||||
text_events_with_usage("hi", 1000),
|
||||
finish_memory_extraction_tool_use_events("ec1"),
|
||||
single_text_events("done"),
|
||||
]);
|
||||
let mut worker = make_worker_with_manifest(EXTRACT_NO_COMPACT_MANIFEST, client).await;
|
||||
|
||||
worker.run_text("first").await.unwrap();
|
||||
assert!(
|
||||
worker.extract_pointer().is_none(),
|
||||
"extract has not run yet — pointer must be None"
|
||||
);
|
||||
|
||||
worker.spawn_post_run_memory_jobs();
|
||||
worker.wait_for_memory_jobs().await;
|
||||
|
||||
assert!(
|
||||
worker.extract_pointer().is_some(),
|
||||
"spawn + wait must complete extract; pointer should be set"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn detached_extract_does_not_fork_session_log() {
|
||||
// Source worker and the cloned memory-task worker share `SegmentState` via
|
||||
// `Arc<_>`. The detached extract advances the entry tally through
|
||||
// `save_extension`; the next `run` must see that same tally so
|
||||
// `ensure_head_or_fork` does not spawn a new session.
|
||||
let client = MockClient::new(vec![
|
||||
text_events_with_usage("hi", 1000),
|
||||
finish_memory_extraction_tool_use_events("ec1"),
|
||||
single_text_events("done"),
|
||||
text_events_with_usage("ok", 1000),
|
||||
]);
|
||||
let mut worker = make_worker_with_manifest(EXTRACT_NO_COMPACT_MANIFEST, client).await;
|
||||
|
||||
worker.run_text("first").await.unwrap();
|
||||
let session_before = worker.segment_id();
|
||||
|
||||
worker.spawn_post_run_memory_jobs();
|
||||
worker.wait_for_memory_jobs().await;
|
||||
|
||||
worker.run_text("second").await.unwrap();
|
||||
let session_after = worker.segment_id();
|
||||
|
||||
assert_eq!(
|
||||
session_before, session_after,
|
||||
"detached extract's save_extension and the next turn's save_user_input \
|
||||
must share the entry tally through SegmentState — a fork here means the \
|
||||
clone carried its own counter"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn controller_compact_method_emits_start_and_done() {
|
||||
let client = MockClient::new(vec![
|
||||
|
||||
Reference in New Issue
Block a user