perf(agent): persistent incremental BM25 index; no store rewrite per query
hybrid_search rebuilt the BM25 index from scratch (re-tokenising every record) and rewrote the whole .h5 file on every single query, so a query cost O(store size) in both CPU and disk I/O. Steady-state p50 per the search harness: 5.5 -> 0.24 ms (1K), 49 -> 2.1 ms (10K), 884 -> 23 ms (100K). - BM25Index is incremental: add_document / remove_document keep it exactly equivalent to a fresh build over the same live documents (property test: 60 random op sequences compared against BM25Index::build after every step). IDF moves to query time since it depends on the live document count. Top-k uses a bounded heap, ties break by doc id (results were HashMap-ordered), and the "WAND" code that computed a bound and then discarded it is removed. - HDF5Memory keeps one index for its lifetime, built lazily. Appends are picked up by ensure_bm25_fresh whatever path added them; delete and in-place update report themselves; compaction drops the index. A test drives every mutation and compares against a fresh build. - A query no longer calls flush(). Activation boosts are marked dirty and persisted by the next checkpoint, including a best-effort one on drop so a search-only session keeps them (approved behaviour change). Activation weights are capped at 16.0; they previously grew without bound. The archived mission branch's BM25 cache was reviewed and not used: it was invalidated by every write, so interleaved save/search still rebuilt per query, and it changed the default fusion weights. Co-Authored-By: Claude Fable 5.1 <[email protected]>
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
61424d1418
commit
2bfbb7fb4b
@@ -205,6 +205,12 @@ pub trait AgentMemory {
|
||||
fn get_session_summary(&self, session_id: &str) -> Result<Option<String>>;
|
||||
}
|
||||
|
||||
/// Ceiling for a record's Hebbian activation weight. Each hit adds
|
||||
/// `hebbian_boost` and the fused score is scaled by `sqrt(weight)`, so without
|
||||
/// a cap a frequently returned record's advantage grows without limit and it
|
||||
/// eventually outranks better matches purely on popularity.
|
||||
pub(crate) const MAX_ACTIVATION_WEIGHT: f32 = 16.0;
|
||||
|
||||
/// Most anomaly alerts kept between `take_anomaly_alerts` calls.
|
||||
const MAX_PENDING_ALERTS: usize = 1024;
|
||||
|
||||
@@ -247,6 +253,14 @@ pub struct HDF5Memory {
|
||||
/// via [`HDF5Memory::take_anomaly_alerts`]. Saves are never blocked on
|
||||
/// these — surfacing is opt-in for callers that want to act on them.
|
||||
anomaly_alerts: Vec<anomaly::AnomalyAlert>,
|
||||
/// Keyword index over `cache.chunks`, kept for the life of the store and
|
||||
/// updated incrementally — it used to be rebuilt from scratch, re-tokenising
|
||||
/// every record, on every single query. Built lazily on first use; see
|
||||
/// [`HDF5Memory::ensure_bm25_fresh`] for how it stays in sync.
|
||||
bm25: Option<bm25::BM25Index>,
|
||||
/// Activation weights changed since the last checkpoint (searches boost
|
||||
/// the records they return). Cleared by `flush`.
|
||||
activations_dirty: bool,
|
||||
/// Opened with [`HDF5Memory::open_read_only`]: nothing may reach the disk.
|
||||
read_only: bool,
|
||||
/// A WAL that `open()` could not read and moved aside; see
|
||||
@@ -299,6 +313,8 @@ impl HDF5Memory {
|
||||
provenance: provenance::ProvenanceStore::new(),
|
||||
anomaly: anomaly::WriteAnomalyDetector::new(anomaly::AnomalyConfig::default()),
|
||||
anomaly_alerts: Vec::new(),
|
||||
bm25: None,
|
||||
activations_dirty: false,
|
||||
read_only: false,
|
||||
quarantined_wal: None,
|
||||
_lock: Some(lock),
|
||||
@@ -434,12 +450,58 @@ impl HDF5Memory {
|
||||
provenance: provenance::ProvenanceStore::new(),
|
||||
anomaly: anomaly::WriteAnomalyDetector::new(anomaly::AnomalyConfig::default()),
|
||||
anomaly_alerts: Vec::new(),
|
||||
bm25: None,
|
||||
activations_dirty: false,
|
||||
read_only,
|
||||
quarantined_wal,
|
||||
_lock: lock,
|
||||
})
|
||||
}
|
||||
|
||||
/// Bring the keyword index in line with the cache and return it.
|
||||
///
|
||||
/// Appends need no hook: records the index hasn't seen yet (whatever path
|
||||
/// added them) are indexed here, in order. Changes that keep the length the
|
||||
/// same are reported explicitly — [`Self::bm25_on_delete`] and
|
||||
/// [`Self::bm25_on_update`] — and anything that renumbers records
|
||||
/// (compaction) drops the index so it is rebuilt.
|
||||
pub(crate) fn ensure_bm25_fresh(&mut self) -> &bm25::BM25Index {
|
||||
let n = self.cache.chunks.len();
|
||||
let bm25 = match self.bm25.take() {
|
||||
Some(index) if index.len() <= n => {
|
||||
let mut index = index;
|
||||
for id in index.len()..n {
|
||||
if self.cache.tombstones[id] == 0 {
|
||||
index.add_document(id, &self.cache.chunks[id]);
|
||||
}
|
||||
}
|
||||
index.pad_to(n);
|
||||
index
|
||||
}
|
||||
_ => bm25::BM25Index::build(&self.cache.chunks, &self.cache.tombstones),
|
||||
};
|
||||
self.bm25.insert(bm25)
|
||||
}
|
||||
|
||||
/// Record `id` was tombstoned; its text is still in the cache.
|
||||
fn bm25_on_delete(&mut self, id: usize) {
|
||||
if let Some(index) = self.bm25.as_mut()
|
||||
&& id < index.len()
|
||||
{
|
||||
index.remove_document(id, &self.cache.chunks[id]);
|
||||
}
|
||||
}
|
||||
|
||||
/// Record `id`'s text changed from `old_text` to what the cache holds now.
|
||||
fn bm25_on_update(&mut self, id: usize, old_text: &str) {
|
||||
if let Some(index) = self.bm25.as_mut()
|
||||
&& id < index.len()
|
||||
{
|
||||
index.remove_document(id, old_text);
|
||||
index.add_document(id, &self.cache.chunks[id]);
|
||||
}
|
||||
}
|
||||
|
||||
/// Flush current state to disk and truncate the WAL.
|
||||
///
|
||||
/// Every code path that persists the full cache to the .h5 file must
|
||||
@@ -466,6 +528,7 @@ impl HDF5Memory {
|
||||
if let Some(ref mut w) = self.wal {
|
||||
w.truncate()?;
|
||||
}
|
||||
self.activations_dirty = false;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -777,6 +840,7 @@ impl HDF5Memory {
|
||||
&entry.session_id,
|
||||
entry.timestamp,
|
||||
);
|
||||
let old_text = std::mem::take(&mut self.cache.chunks[existing_idx]);
|
||||
self.cache.update(
|
||||
existing_idx,
|
||||
entry.chunk,
|
||||
@@ -785,6 +849,7 @@ impl HDF5Memory {
|
||||
entry.timestamp,
|
||||
entry.session_id,
|
||||
);
|
||||
self.bm25_on_update(existing_idx, &old_text);
|
||||
// In-place embedding change: the index node is stale, force rebuild.
|
||||
self.hnsw_mark_dirty();
|
||||
let needs_flush = self
|
||||
@@ -876,6 +941,7 @@ impl AgentMemory for HDF5Memory {
|
||||
)));
|
||||
}
|
||||
self.hnsw_on_delete(id);
|
||||
self.bm25_on_delete(id);
|
||||
self.flush()?;
|
||||
|
||||
// Auto-compact if threshold exceeded
|
||||
@@ -893,6 +959,7 @@ impl AgentMemory for HDF5Memory {
|
||||
if removed > 0 {
|
||||
// Record ids are cache indices, which compaction just renumbered.
|
||||
self.provenance.remap(&index_map);
|
||||
self.bm25 = None;
|
||||
// Compaction renumbers cache indices; rebuild the index to match.
|
||||
self.hnsw_mark_dirty();
|
||||
self.flush()?;
|
||||
@@ -1172,6 +1239,18 @@ impl HDF5Memory {
|
||||
|
||||
// --- Tests ---
|
||||
|
||||
impl Drop for HDF5Memory {
|
||||
/// Best-effort checkpoint of activation weights that only searches have
|
||||
/// touched. Everything else is already durable through the WAL or an
|
||||
/// earlier checkpoint; without this a search-only session would forget
|
||||
/// every boost it made.
|
||||
fn drop(&mut self) {
|
||||
if self.activations_dirty && !self.read_only {
|
||||
let _ = self.flush();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -1595,6 +1674,83 @@ mod tests {
|
||||
assert_eq!(restored.cache.chunks, ["checkpointed", "wal-only"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keyword_index_stays_in_sync_through_every_mutation() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let mut config = make_config(&dir);
|
||||
config.compact_threshold = 0.0; // compact only when asked
|
||||
let mut mem = HDF5Memory::create(config).unwrap();
|
||||
let check = |mem: &mut HDF5Memory, what: &str| {
|
||||
let fresh = bm25::BM25Index::build(&mem.cache.chunks, &mem.cache.tombstones);
|
||||
let n = mem.cache.len();
|
||||
for query in ["apple", "banana cherry", "date", "nothing"] {
|
||||
let kept = mem.ensure_bm25_fresh().search(query, n);
|
||||
assert_eq!(kept, fresh.search(query, n), "{what}: {query:?}");
|
||||
}
|
||||
};
|
||||
let tagged = |chunk: &str, tag: &str| {
|
||||
let mut e = make_entry(chunk, &[1.0, 0.0, 0.0, 0.0]);
|
||||
e.tags = tag.into();
|
||||
e
|
||||
};
|
||||
|
||||
check(&mut mem, "empty");
|
||||
mem.save(tagged("apple banana", "a")).unwrap();
|
||||
mem.save(tagged("banana cherry cherry", "b")).unwrap();
|
||||
check(&mut mem, "after saves");
|
||||
mem.save_batch(vec![tagged("date apple", "c"), tagged("cherry", "d")])
|
||||
.unwrap();
|
||||
check(&mut mem, "after save_batch");
|
||||
mem.save_or_update(tagged("date date date", "a")).unwrap();
|
||||
check(&mut mem, "after in-place update");
|
||||
mem.delete(1).unwrap();
|
||||
check(&mut mem, "after delete");
|
||||
mem.save(tagged("apple cherry", "e")).unwrap();
|
||||
check(&mut mem, "after save following a delete");
|
||||
mem.compact().unwrap();
|
||||
check(&mut mem, "after compact");
|
||||
mem.hybrid_search(&[1.0, 0.0, 0.0, 0.0], "apple", 0.5, 0.5, 3);
|
||||
check(&mut mem, "after a search");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn search_does_not_write_the_store_but_boosts_persist_on_drop() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let config = make_config(&dir);
|
||||
let path = config.path.clone();
|
||||
let mut mem = HDF5Memory::create(config).unwrap();
|
||||
mem.save(make_entry("findable", &[1.0, 0.0, 0.0, 0.0]))
|
||||
.unwrap();
|
||||
let before = std::fs::read(&path).unwrap();
|
||||
|
||||
for _ in 0..3 {
|
||||
mem.hybrid_search(&[1.0, 0.0, 0.0, 0.0], "findable", 1.0, 0.0, 1);
|
||||
}
|
||||
assert_eq!(
|
||||
std::fs::read(&path).unwrap(),
|
||||
before,
|
||||
"a query must not rewrite the store"
|
||||
);
|
||||
let boosted = mem.cache.activation_weights[0];
|
||||
assert!(boosted > 1.0);
|
||||
drop(mem);
|
||||
|
||||
let reopened = HDF5Memory::open(&path).unwrap();
|
||||
assert_eq!(reopened.cache.activation_weights[0], boosted);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn activation_weight_is_capped() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let mut mem = HDF5Memory::create(make_config(&dir)).unwrap();
|
||||
mem.save(make_entry("popular", &[1.0, 0.0, 0.0, 0.0]))
|
||||
.unwrap();
|
||||
for _ in 0..500 {
|
||||
mem.hybrid_search(&[1.0, 0.0, 0.0, 0.0], "popular", 1.0, 0.0, 1);
|
||||
}
|
||||
assert_eq!(mem.cache.activation_weights[0], MAX_ACTIVATION_WEIGHT);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn store_has_a_single_writer() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
|
||||
Reference in New Issue
Block a user