Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .cargo/mutants.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,23 @@ test_tool = "nextest"
# and `<` mutants on the same comparison are caught).
exclude_re = [
"replace > with >= in dispatch_chunks_concurrently",
# `compact_passages_if_worthwhile`'s watermark guard (issue #587):
# `PassageStore::load` only ever produces `watermark() == 0`
# together with `log_bytes == 0` and `snapshot_bytes == 0` — a
# fresh store, or one healed back to a clean state, never a store
# with pending log bytes and no watermark. With both at zero,
# `compact_if_log_outgrew_snapshot`'s own condition
# (`log_bytes > floor_bytes.max(ratio * snapshot_bytes)`) is
# `0 > floor_bytes.max(0)`, false for every `floor_bytes: u64`
# regardless of this guard — no test can observe a difference
# because there is none to observe.
"replace match guard store\\.watermark\\(\\) > 0 with true in AppState::compact_passages_if_worthwhile",
# Same guard, the `>` -> `>=` operator mutation specifically:
# `watermark(): u64` can never be negative, so `watermark() >= 0`
# is a tautology — identical to `true` for every possible value,
# not just zero. Same equivalence as the entry above, different
# mutation operator on the same expression.
"replace > with >= in AppState::compact_passages_if_worthwhile",
# The in-process embedding provider (local-embed, on by default)
# is exercised by an #[ignore]d smoke test that downloads a real
# model — CI sweeps can never kill mutants there, and counting
Expand Down
154 changes: 153 additions & 1 deletion src/registry/context_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ mod tests {
AliasInput, AssocInput, RetractionInput, config as proptest_config,
json_roundtrip_f64_strategy, scenario_strategy,
};
use crate::registry::test_support::{assoc_op, scratch_dir};
use crate::registry::test_support::{assoc_op, plain, scratch_dir};
use proptest::prelude::*;

/// The three revision counters move on exactly their own lane —
Expand Down Expand Up @@ -838,6 +838,158 @@ mod tests {
let _ = fs::remove_dir_all(dir);
}

/// `compact_passages_if_worthwhile` (taguru-code's sync, issue #452)
/// has never had a single test in this repository despite four
/// early-return branches — issue #587. A replica's snapshot and log
/// are manifest-owned, so it must decline before ever touching the
/// entry's passage store.
#[test]
fn compact_passages_if_worthwhile_never_compacts_on_a_replica() {
let dir = scratch_dir("compact-worthwhile-replica");
{
let state = AppState::boot(dir.clone(), usize::MAX, None).unwrap();
state.create("sake", ContextMeta::default()).unwrap();
state
.store_passages(
"sake",
plain(BTreeMap::from([(
"a.md".to_string(),
"蔵は1832年創業。".to_string(),
)])),
)
.unwrap()
.unwrap();
}
let wal_path = passages_wal_path(&dir, &file_stem("sake"));
let before = fs::metadata(&wal_path).unwrap().len();
assert!(before > 0, "sanity: the store has a pending log");

let state = AppState::boot_with(
dir.clone(),
usize::MAX,
None,
BootOptions {
replica: Some(std::sync::Arc::new(crate::replica::ReplicaInfo::new(None))),
..BootOptions::default()
},
)
.unwrap();
assert!(state.is_replica(), "sanity: the reopened boot is a replica");

assert!(
!state.compact_passages_if_worthwhile("sake", 0),
"a replica must decline even with a floor of zero"
);
assert_eq!(
fs::metadata(&wal_path).unwrap().len(),
before,
"a declined replica compaction must leave the log untouched"
);
let _ = fs::remove_dir_all(dir);
}

/// An unknown context name is the second early return.
#[test]
fn compact_passages_if_worthwhile_declines_an_unknown_context() {
let dir = scratch_dir("compact-worthwhile-unknown");
let state = AppState::boot(dir.clone(), usize::MAX, None).unwrap();
assert!(!state.compact_passages_if_worthwhile("no-such-context", 0));
let _ = fs::remove_dir_all(dir);
}

/// A deleted (tombstoned) context is the third early return —
/// `read_unless_deleted` must refuse it just as it does every other
/// entry point.
#[test]
fn compact_passages_if_worthwhile_declines_a_deleted_context() {
let dir = scratch_dir("compact-worthwhile-deleted");
let state = AppState::boot(dir.clone(), usize::MAX, None).unwrap();
state.create("sake", ContextMeta::default()).unwrap();
// Simulates the race `delete()` can open: still a member of the
// registry map (so `lookup` finds it, exercising the
// `read_unless_deleted()` branch rather than `lookup`'s own
// `None` one) but its slot already flipped to the tombstone —
// same shape as `run_maintenance_compaction_skips_a_deleted_entry_without_panicking`
// above.
let entry = state.lookup("sake").expect("just created");
entry.inner.write().slot = Slot::Deleted;
assert!(!state.compact_passages_if_worthwhile("sake", 0));
let _ = fs::remove_dir_all(dir);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

/// The watermark guard: a context whose passage log has never
/// received a single write must not get store files minted just to
/// compact nothing.
#[test]
fn compact_passages_if_worthwhile_declines_a_context_with_no_passage_history() {
let dir = scratch_dir("compact-worthwhile-watermark-zero");
let state = AppState::boot(dir.clone(), usize::MAX, None).unwrap();
state.create("sake", ContextMeta::default()).unwrap();

assert!(!state.compact_passages_if_worthwhile("sake", 0));
assert!(
!passages_path(&dir, &file_stem("sake")).exists(),
"a declined compaction on an empty store must write nothing"
);
assert!(
!passages_wal_path(&dir, &file_stem("sake")).exists(),
"a store that was never written to must never be touched on disk"
);
let _ = fs::remove_dir_all(dir);
}

/// The one path where `compact_passages_if_worthwhile` actually
/// runs: passage history past the floor folds the log into the
/// snapshot, same as the write-path trigger it stands in for.
#[test]
fn compact_passages_if_worthwhile_compacts_past_the_floor() {
let dir = scratch_dir("compact-worthwhile-runs");
let state = AppState::boot(dir.clone(), usize::MAX, None).unwrap();
state.create("sake", ContextMeta::default()).unwrap();
state
.store_passages(
"sake",
plain(BTreeMap::from([(
"a.md".to_string(),
"蔵は1832年創業。".to_string(),
)])),
)
.unwrap()
.unwrap();

let wal_path = passages_wal_path(&dir, &file_stem("sake"));
let pending = fs::metadata(&wal_path).unwrap().len();
assert!(pending > 0, "sanity: the store has a pending log");

assert!(
state.compact_passages_if_worthwhile("sake", pending - 1),
"a log past the floor is worthwhile to fold"
);
assert_eq!(
fs::metadata(&wal_path).unwrap().len(),
0,
"the covered log truncates once compacted"
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// An emptied log alone doesn't prove the passage survived —
// an implementation that truncates the WAL before the snapshot
// actually lands would pass the assertion above while losing
// the write. Restart from disk and dereference it back.
drop(state);
let reopened = AppState::boot(dir.clone(), usize::MAX, None).unwrap();
let (passages, missing) = reopened
.lookup_passages("sake", &["a.md".to_string()])
.unwrap()
.unwrap();
assert!(missing.is_empty(), "{missing:?}");
assert_eq!(
passages.get("a.md").map(String::as_str),
Some("蔵は1832年創業。")
);

let _ = fs::remove_dir_all(dir);
}

/// Compaction sheds the dead weight retraction leaves behind,
/// preserves every live fact, keeps the WAL watermark monotonic —
/// a write after the compact replays correctly across a hard crash
Expand Down