update: manifestで一部値のzeroの扱いを変更
This commit is contained in:
parent
31d9b9b2b7
commit
00755cf1b8
|
|
@ -1333,7 +1333,9 @@ impl<C: LlmClient, St: Store> Pod<C, St> {
|
|||
let Some(memory_cfg) = self.manifest.memory.clone() else {
|
||||
return Ok(());
|
||||
};
|
||||
let Some(threshold) = memory_cfg.extract_threshold else {
|
||||
// `Some(0)` means disabled, same as `None`. Otherwise the
|
||||
// `tokens_since >= 0` comparison would fire on every post-run.
|
||||
let Some(threshold) = memory_cfg.extract_threshold.filter(|n| *n > 0) else {
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
|
|
@ -1538,8 +1540,13 @@ impl<C: LlmClient, St: Store> Pod<C, St> {
|
|||
let Some(memory_cfg) = self.manifest.memory.clone() else {
|
||||
return Ok(());
|
||||
};
|
||||
let files_threshold = memory_cfg.consolidation_threshold_files;
|
||||
let bytes_threshold = memory_cfg.consolidation_threshold_bytes;
|
||||
// `Some(0)` collapses to `None` — staging count / bytes always
|
||||
// satisfies `>= 0`, which would fire Phase 2 on every post-run.
|
||||
// Treating zero as disabled lines up with `extract_threshold` and
|
||||
// matches the "no threshold ⇒ Phase 2 off" invariant in the
|
||||
// ticket's §Trigger.
|
||||
let files_threshold = memory_cfg.consolidation_threshold_files.filter(|n| *n > 0);
|
||||
let bytes_threshold = memory_cfg.consolidation_threshold_bytes.filter(|n| *n > 0);
|
||||
if files_threshold.is_none() && bytes_threshold.is_none() {
|
||||
return Ok(());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -368,6 +368,49 @@ async fn compact_resets_extract_pointer_so_phase1_can_fire_again() {
|
|||
);
|
||||
}
|
||||
|
||||
/// `extract_threshold = 0` is treated as "disabled" — without this, a
|
||||
/// raw `>=` comparison against `tokens_since` would fire Phase 1 on
|
||||
/// every post-run regardless of activity. Mirrors the Phase 2
|
||||
/// zero-threshold convention so users have a single way to opt out
|
||||
/// without removing the `[memory]` section.
|
||||
const EXTRACT_THRESHOLD_ZERO_MANIFEST: &str = r#"
|
||||
[pod]
|
||||
name = "test-pod"
|
||||
pwd = "./"
|
||||
|
||||
[model]
|
||||
scheme = "anthropic"
|
||||
model_id = "test-model"
|
||||
|
||||
[worker]
|
||||
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 Phase 1
|
||||
// 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 pod = make_pod_with_manifest(EXTRACT_THRESHOLD_ZERO_MANIFEST, client).await;
|
||||
|
||||
pod.run_text("first").await.unwrap();
|
||||
pod.try_post_run_extract()
|
||||
.await
|
||||
.expect("extract_threshold=0 must skip silently, not fail");
|
||||
assert!(
|
||||
pod.extract_pointer().is_none(),
|
||||
"no extract should have run — pointer must remain None"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn post_run_compact_failure_broadcasts_start_and_failed() {
|
||||
// Only the first run has a response. Compaction will run the
|
||||
|
|
|
|||
|
|
@ -130,6 +130,26 @@ target = "./"
|
|||
permission = "write"
|
||||
"#;
|
||||
|
||||
const ZERO_THRESHOLDS_TOML: &str = r#"
|
||||
[pod]
|
||||
name = "test-pod"
|
||||
|
||||
[model]
|
||||
scheme = "anthropic"
|
||||
model_id = "test-model"
|
||||
|
||||
[worker]
|
||||
max_tokens = 100
|
||||
|
||||
[memory]
|
||||
consolidation_threshold_files = 0
|
||||
consolidation_threshold_bytes = 0
|
||||
|
||||
[[scope.allow]]
|
||||
target = "./"
|
||||
permission = "write"
|
||||
"#;
|
||||
|
||||
async fn make_pod_with(
|
||||
manifest_toml: &str,
|
||||
pwd: std::path::PathBuf,
|
||||
|
|
@ -192,6 +212,30 @@ async fn no_thresholds_is_a_noop() {
|
|||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn zero_thresholds_treated_as_disabled() {
|
||||
// Without the `Some(0) → None` collapse, `total_files >= 0` and
|
||||
// `total_bytes >= 0` would always evaluate true and Phase 2 would
|
||||
// fire on every post-run with any staging activity.
|
||||
let pwd = tempfile::tempdir().unwrap();
|
||||
let layout = WorkspaceLayout::new(pwd.path().to_path_buf());
|
||||
write_n_staging(&layout, 5);
|
||||
|
||||
let client = MockClient::new(vec![]);
|
||||
let mut pod = make_pod_with(ZERO_THRESHOLDS_TOML, pwd.path().to_path_buf(), client).await;
|
||||
pod.try_post_run_consolidate()
|
||||
.await
|
||||
.expect("zero thresholds must collapse to disabled, not fire on every staging entry");
|
||||
|
||||
assert_eq!(
|
||||
memory::consolidate::list_staging_entries(&layout).len(),
|
||||
5,
|
||||
"staging must be untouched when both thresholds are zero"
|
||||
);
|
||||
let lock_path = layout.staging_dir().join(".consolidation.lock");
|
||||
assert!(!lock_path.exists(), "no lock should be acquired");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn empty_staging_skips() {
|
||||
let pwd = tempfile::tempdir().unwrap();
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user