From 7ee2cf26cc9e97dd22b39543e2cda3b1c4cee7e9 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 23 Aug 2026 22:16:45 +0200 Subject: [PATCH 1/3] Optimise touched-node test sweep (#47) Track adjacency lists changed by test-only graph mutations so the post-mutation reciprocity repair no longer scans unrelated graph edges. Keep deletion and reachability repairs in the same tracking boundary, and retain focused tests that prove localized repair leaves unrelated edges untouched. --- chutoro-core/src/hnsw/cpu/test_helpers.rs | 12 +- chutoro-core/src/hnsw/graph/core.rs | 7 ++ .../src/hnsw/graph/test_helpers/mod.rs | 103 ++++++++++----- .../src/hnsw/graph/test_helpers/tests.rs | 6 + chutoro-core/src/hnsw/insert/executor.rs | 11 +- .../src/hnsw/insert/executor/tests/mod.rs | 24 ++++ chutoro-core/src/hnsw/insert/test_helpers.rs | 119 +++++++++++++----- docs/adr-001-commit-post-processing.md | 9 ++ 8 files changed, 227 insertions(+), 64 deletions(-) diff --git a/chutoro-core/src/hnsw/cpu/test_helpers.rs b/chutoro-core/src/hnsw/cpu/test_helpers.rs index a59dc2e7..7e11f6ad 100644 --- a/chutoro-core/src/hnsw/cpu/test_helpers.rs +++ b/chutoro-core/src/hnsw/cpu/test_helpers.rs @@ -17,9 +17,15 @@ impl CpuHnsw { /// property-based mutation checks that rely on post-commit healing passes. pub fn heal_for_test(&self) { let healed = self.write_graph(|graph| { - let mut executor = graph.insertion_executor(); - executor.heal_reachability(self.params.max_connections()); - executor.enforce_bidirectional_all(self.params.max_connections()); + { + let mut executor = graph.insertion_executor(); + executor.heal_reachability(self.params.max_connections()); + } + let touched = graph.take_touched_nodes(); + { + let mut executor = graph.insertion_executor(); + executor.enforce_bidirectional_for_touched(&touched, self.params.max_connections()); + } Ok(()) }); if let Err(err) = healed { diff --git a/chutoro-core/src/hnsw/graph/core.rs b/chutoro-core/src/hnsw/graph/core.rs index 833337f8..28302771 100644 --- a/chutoro-core/src/hnsw/graph/core.rs +++ b/chutoro-core/src/hnsw/graph/core.rs @@ -9,6 +9,9 @@ use crate::hnsw::{ types::{EntryPoint, InsertionPlan}, }; +#[cfg(test)] +use std::collections::BTreeSet; + /// Context for attaching or inserting a node into the HNSW graph. /// /// The insertion `sequence` is used for deterministic neighbour ordering and @@ -168,6 +171,8 @@ pub(crate) struct Graph { pub(super) params: HnswParams, pub(super) nodes: Vec>, pub(super) entry: Option, + #[cfg(test)] + pub(super) touched: BTreeSet<(usize, usize)>, } fn should_promote_entry(current: Option, level: usize) -> bool { @@ -183,6 +188,8 @@ impl Graph { params, nodes: vec![None; capacity], entry: None, + #[cfg(test)] + touched: BTreeSet::new(), } } diff --git a/chutoro-core/src/hnsw/graph/test_helpers/mod.rs b/chutoro-core/src/hnsw/graph/test_helpers/mod.rs index b2d48d93..1204c699 100644 --- a/chutoro-core/src/hnsw/graph/test_helpers/mod.rs +++ b/chutoro-core/src/hnsw/graph/test_helpers/mod.rs @@ -19,6 +19,19 @@ use crate::hnsw::{ mod tests; impl Graph { + /// Records test-only mutation pairs for the next localized healing pass. + pub(crate) fn record_touched_nodes(&mut self, touched: I) + where + I: IntoIterator, + { + self.touched.extend(touched); + } + + /// Returns and clears the mutation pairs accumulated since the last pass. + pub(crate) fn take_touched_nodes(&mut self) -> Vec<(usize, usize)> { + std::mem::take(&mut self.touched).into_iter().collect() + } + pub(crate) fn set_params(&mut self, params: &HnswParams) { self.params = params.clone(); } @@ -37,7 +50,8 @@ impl Graph { unreachable!("node presence checked above"); }; - self.strip_references_to(node); + let removed_references = self.strip_references_to(node); + self.record_touched_nodes(removed_references); self.reconnect_layers(removed_neighbours); if self.entry.map(|entry| entry.node) == Some(node) { @@ -127,37 +141,50 @@ impl Graph { pub(super) fn try_add_edge(&mut self, origin: usize, target: usize, level: usize) -> bool { let limit = params::connection_limit_for_level(level, self.params.max_connections()); - let Some(node) = self.nodes.get_mut(origin).and_then(Option::as_mut) else { - return false; - }; - if level >= node.level_count() { - return false; - } + let added = { + let Some(node) = self.nodes.get_mut(origin).and_then(Option::as_mut) else { + return false; + }; + if level >= node.level_count() { + return false; + } - let neighbours = node.neighbours_mut(level); - if neighbours.contains(&target) { - return true; - } + let neighbours = node.neighbours_mut(level); + if neighbours.contains(&target) { + return true; + } + + if neighbours.len() >= limit { + return false; + } - if neighbours.len() < limit { neighbours.push(target); - return true; + true + }; + if added { + self.record_touched_nodes([(origin, level)]); } - - false + added } pub(super) fn remove_edge(&mut self, origin: usize, target: usize, level: usize) { - let Some(node) = self.nodes.get_mut(origin).and_then(Option::as_mut) else { - return; - }; - if level >= node.level_count() { - return; - } + let removed = { + let Some(node) = self.nodes.get_mut(origin).and_then(Option::as_mut) else { + return; + }; + if level >= node.level_count() { + return; + } - let neighbours = node.neighbours_mut(level); - if let Some(pos) = neighbours.iter().position(|&candidate| candidate == target) { + let neighbours = node.neighbours_mut(level); + let Some(pos) = neighbours.iter().position(|&candidate| candidate == target) else { + return; + }; neighbours.remove(pos); + true + }; + if removed { + self.record_touched_nodes([(origin, level)]); } } @@ -170,14 +197,30 @@ impl Graph { Ok(()) } - fn strip_references_to(&mut self, node: usize) { - for maybe_node in self.nodes.iter_mut().flatten() { - let levels = maybe_node.level_count(); - for level in 0..levels { - let neighbours = maybe_node.neighbours_mut(level); - neighbours.retain(|&target| target != node); - } + fn strip_references_to(&mut self, node: usize) -> Vec<(usize, usize)> { + let mut touched = Vec::new(); + for (id, maybe_node) in self.nodes.iter_mut().enumerate() { + let Some(existing) = maybe_node.as_mut() else { + continue; + }; + touched.extend(Self::strip_node_references(id, existing, node)); } + touched + } + + fn strip_node_references( + id: usize, + existing: &mut crate::hnsw::node::Node, + target: usize, + ) -> Vec<(usize, usize)> { + (0..existing.level_count()) + .filter_map(|level| { + let neighbours = existing.neighbours_mut(level); + let previous_len = neighbours.len(); + neighbours.retain(|&candidate| candidate != target); + (neighbours.len() != previous_len).then_some((id, level)) + }) + .collect() } fn reconnect_layers(&mut self, removed_neighbours: Vec>) { diff --git a/chutoro-core/src/hnsw/graph/test_helpers/tests.rs b/chutoro-core/src/hnsw/graph/test_helpers/tests.rs index a35d8f0b..a186f03f 100644 --- a/chutoro-core/src/hnsw/graph/test_helpers/tests.rs +++ b/chutoro-core/src/hnsw/graph/test_helpers/tests.rs @@ -53,6 +53,7 @@ fn delete_node_reconnects_neighbours_and_preserves_reachability(mut small_graph: .expect("attach second neighbour"); small_graph.try_add_bidirectional_edge(0, 1, 0); small_graph.try_add_bidirectional_edge(1, 2, 0); + let _ = small_graph.take_touched_nodes(); let deleted = small_graph.delete_node(1).expect("delete must succeed"); @@ -66,6 +67,11 @@ fn delete_node_reconnects_neighbours_and_preserves_reachability(mut small_graph: let node2 = small_graph.node(2).expect("node 2 must remain"); assert_eq!(node2.neighbours(0), &[0], "node 2 must connect to node 0"); assert_eq!(small_graph.entry().map(|entry| entry.node), Some(0)); + assert_eq!( + small_graph.take_touched_nodes(), + vec![(0, 0), (2, 0)], + "delete must queue only adjacency lists that it changed", + ); } #[rstest] diff --git a/chutoro-core/src/hnsw/insert/executor.rs b/chutoro-core/src/hnsw/insert/executor.rs index c78d9cf7..43aecd0b 100644 --- a/chutoro-core/src/hnsw/insert/executor.rs +++ b/chutoro-core/src/hnsw/insert/executor.rs @@ -154,6 +154,9 @@ impl<'graph> InsertionExecutor<'graph> { touched.extend((0..=new_node.level).map(|level| (new_node.id, level))); + #[cfg(test)] + self.graph.record_touched_nodes(touched.iter().copied()); + #[cfg(any(test, debug_assertions))] { let auditor = ReciprocityAuditor::new(self.graph); @@ -234,9 +237,13 @@ impl<'graph> InsertionExecutor<'graph> { } #[cfg(test)] - pub(crate) fn enforce_bidirectional_all(&mut self, max_connections: usize) { + pub(crate) fn enforce_bidirectional_for_touched( + &mut self, + touched: &[(usize, usize)], + max_connections: usize, + ) { super::test_helpers::TestHelpers::new(self.graph) - .enforce_bidirectional_all(max_connections); + .enforce_bidirectional_for_touched(touched, max_connections); } } diff --git a/chutoro-core/src/hnsw/insert/executor/tests/mod.rs b/chutoro-core/src/hnsw/insert/executor/tests/mod.rs index 01d5cc0d..b760d146 100644 --- a/chutoro-core/src/hnsw/insert/executor/tests/mod.rs +++ b/chutoro-core/src/hnsw/insert/executor/tests/mod.rs @@ -235,6 +235,30 @@ fn enforce_bidirectional_all_removes_invalid_upper_edge() { assert_no_edge(&graph, 1, 0, 1); } +#[test] +fn enforce_bidirectional_for_touched_leaves_unrelated_edges_unchanged() { + let mut graph = setup_basic_graph(2, 4, 3).expect("params should be valid in tests"); + insert_entry_node(&mut graph, 0).expect("insert entry"); + attach_test_node(&mut graph, 1, 0, 1).expect("attach first node"); + attach_test_node(&mut graph, 2, 0, 2).expect("attach second node"); + + add_edge_if_missing(&mut graph, 0, 1, 0); + add_edge_if_missing(&mut graph, 2, 0, 0); + + TestHelpers::new(&mut graph).enforce_bidirectional_for_touched(&[(0, 0)], 2); + + assert_bidirectional_edge(&graph, 0, 1, 0); + assert_no_edge(&graph, 0, 2, 0); + assert!( + graph + .node(2) + .expect("untracked node must remain") + .neighbours(0) + .contains(&0), + "untracked edge must not be processed", + ); +} + #[rstest] #[case::evicts_tail(vec![1, 3], 1)] #[case::evicts_tail_wider(vec![1, 3, 4, 5], 2)] diff --git a/chutoro-core/src/hnsw/insert/test_helpers.rs b/chutoro-core/src/hnsw/insert/test_helpers.rs index e853b42c..51ea08a8 100644 --- a/chutoro-core/src/hnsw/insert/test_helpers.rs +++ b/chutoro-core/src/hnsw/insert/test_helpers.rs @@ -101,6 +101,7 @@ impl<'graph> TestHelpers<'graph> { }; let mut healer = ConnectivityHealer::new(self.graph); if healer.link_new_node(&ctx, node_id) { + self.graph.record_touched_nodes([(origin, 0), (node_id, 0)]); return true; } } @@ -113,6 +114,7 @@ impl<'graph> TestHelpers<'graph> { }; let mut healer = ConnectivityHealer::new(self.graph); if healer.link_new_node(&ctx, node_id) { + self.graph.record_touched_nodes([(origin, 0), (node_id, 0)]); return true; } } @@ -189,6 +191,24 @@ impl<'graph> TestHelpers<'graph> { self.validate_all_edges_reciprocal(max_connections); } + /// Repairs and validates only edges owned by graph nodes changed by a test mutation. + pub(super) fn enforce_bidirectional_for_touched( + &mut self, + touched: &[(usize, usize)], + max_connections: usize, + ) { + for (origin, level, target) in self.collect_touched_edges(touched) { + let ctx = UpdateContext { + origin, + level, + max_connections, + }; + self.heal_or_remove_edge(&ctx, target); + } + + self.validate_touched_edges_reciprocal(touched, max_connections); + } + pub(super) fn collect_edges(&self) -> Vec<(usize, usize, usize)> { self.graph .nodes_iter() @@ -199,6 +219,25 @@ impl<'graph> TestHelpers<'graph> { .collect() } + fn collect_touched_edges(&self, touched: &[(usize, usize)]) -> Vec<(usize, usize, usize)> { + let mut edges = Vec::new(); + for &(origin, level) in touched { + let Some(node) = self.graph.node(origin) else { + continue; + }; + if level >= node.level_count() { + continue; + } + edges.extend( + node.neighbours(level) + .iter() + .copied() + .map(|target| (origin, level, target)), + ); + } + edges + } + pub(super) fn heal_or_remove_edge(&mut self, ctx: &UpdateContext, target: usize) { if let Some(target_node) = self.graph.node_mut(target) && ctx.level < target_node.level_count() @@ -219,36 +258,58 @@ impl<'graph> TestHelpers<'graph> { reconciler.remove_forward_edge_from(ctx, target); } - #[expect( - clippy::excessive_nesting, - reason = "test-only reciprocal validation keeps explicit panic messages" - )] pub(super) fn validate_all_edges_reciprocal(&self, max_connections: usize) { - for (origin, node) in self.graph.nodes_iter() { - for (level, target) in node.iter_neighbours() { - let target_node = match self.graph.node(target) { - Some(node) => node, - None => { - panic!( - "enforce_bidirectional_all left edge {origin}->{target} at level {level} to missing node", - ); - } - }; - - let target_levels = target_node.level_count(); - assert!( - level < target_levels, - "enforce_bidirectional_all left edge {origin}->{target} at absent level {level} (target has {target_levels})", - ); - - let neighbours = target_node.neighbours(level); - let limit = compute_connection_limit(level, max_connections); - assert!( - neighbours.contains(&origin), - "enforce_bidirectional_all left one-way edge {origin}->{target} at level {level}; target degree {} (limit {limit})", - neighbours.len(), - ); - } + for (origin, level, target) in self.collect_edges() { + let target_node = match self.graph.node(target) { + Some(node) => node, + None => { + panic!( + "enforce_bidirectional_all left edge {origin}->{target} at level {level} to missing node", + ); + } + }; + + let target_levels = target_node.level_count(); + assert!( + level < target_levels, + "enforce_bidirectional_all left edge {origin}->{target} at absent level {level} (target has {target_levels})", + ); + + let neighbours = target_node.neighbours(level); + let limit = compute_connection_limit(level, max_connections); + assert!( + neighbours.contains(&origin), + "enforce_bidirectional_all left one-way edge {origin}->{target} at level {level}; target degree {} (limit {limit})", + neighbours.len(), + ); + } + } + + fn validate_touched_edges_reciprocal( + &self, + touched: &[(usize, usize)], + max_connections: usize, + ) { + for (origin, level, target) in self.collect_touched_edges(touched) { + let target_node = match self.graph.node(target) { + Some(node) => node, + None => panic!( + "enforce_bidirectional_for_touched left edge {origin}->{target} at level {level} to missing node", + ), + }; + let target_levels = target_node.level_count(); + assert!( + level < target_levels, + "enforce_bidirectional_for_touched left edge {origin}->{target} at absent level {level} (target has {target_levels})", + ); + + let neighbours = target_node.neighbours(level); + let limit = compute_connection_limit(level, max_connections); + assert!( + neighbours.contains(&origin), + "enforce_bidirectional_for_touched left one-way edge {origin}->{target} at level {level}; target degree {} (limit {limit})", + neighbours.len(), + ); } } } diff --git a/docs/adr-001-commit-post-processing.md b/docs/adr-001-commit-post-processing.md index ca266311..19086a63 100644 --- a/docs/adr-001-commit-post-processing.md +++ b/docs/adr-001-commit-post-processing.md @@ -15,6 +15,9 @@ Accepted fix now propagates updates via `Graph::set_params`. - Initial population calculation was capped to half the fixture size to avoid overshooting insert capacity. +- The test-only healing hook currently scans every graph edge after bootstrap + and after each mutation, even though insertion and deletion already know + which adjacency lists they changed. ## Decision @@ -30,6 +33,12 @@ Accepted `CpuHnsw::delete_node_for_test`, scrubbing references, reconnecting former neighbours, recomputing the entry point, and decrementing the public length counter so mutation properties exercise real delete semantics. +- Keep a test-only, graph-owned set of changed `(node, level)` pairs. Commit, + deletion, reconnection, and reachability repair append only lists that they + alter; `CpuHnsw::heal_for_test` drains the set and applies reciprocity repair + and validation only to their outgoing edges. The set is private to graph + mutation helpers: callers neither add arbitrary pairs nor retain a drained + batch for a later mutation. ## Consequences From 30d57558afec9e82e7fccd52ef2ae38be42e5d0a Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 24 Aug 2026 14:20:21 +0200 Subject: [PATCH 2/3] Document localized HNSW healing boundary (#47) Explain the ownership and lifecycle of the test-only touched-node queue in the developers guide and record the rollback invariant in a dated ADR addendum. Keep maintainer guidance aligned with the localized reciprocity sweep. --- docs/adr-001-commit-post-processing.md | 13 +++++++++++++ docs/developers-guide.md | 16 ++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/docs/adr-001-commit-post-processing.md b/docs/adr-001-commit-post-processing.md index 19086a63..733ef970 100644 --- a/docs/adr-001-commit-post-processing.md +++ b/docs/adr-001-commit-post-processing.md @@ -138,3 +138,16 @@ Accepted insertion latency improvements and to guard against degree-bound regressions. - Expand deletion coverage with adversarial cases to validate the lightweight reconnection heuristic under high churn. + +## Addendum — 2026-08-24 + +Issue #47 confirms the localized reciprocity sweep as a test-only boundary. The +graph owns a private queue of changed `(node, level)` pairs; commit, deletion, +reconnection, and reachability repair append the adjacency lists they alter. +`CpuHnsw::heal_for_test` repairs reachability first, then drains the queue once +and enforces reciprocity only for the queued outgoing lists. Production paths +do not expose or depend on this queue. + +Mutation rollback must restore the queue together with the graph nodes and +entry point. This prevents a failed deletion from leaving stale pairs for a +later healing pass and keeps the healing state consistent with the graph. diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 722bc505..deac01af 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -581,6 +581,22 @@ contracts: These contracts let `hnsw/validate.rs` and `hnsw/helpers.rs` merge cache hits and misses without corrupting caller buffers after a provider error. +## Test-only HNSW mutation healing + +The mutation property harness is the only consumer of the graph-owned, +test-only touched-node queue. Mutation helpers record a `(node, level)` pair +when they change that adjacency list during commit, deletion, reconnection, or +reachability repair. `CpuHnsw::heal_for_test` repairs reachability first, then +drains the queue and enforces and validates reciprocity only for the queued +outgoing adjacency lists. This boundary keeps the localized sweep out of +production builds and leaves unrelated adjacency lists untouched. + +When changing mutation helpers, keep the queue aligned with graph state: +record only changed lists, do not let callers add arbitrary pairs, and restore +the queue alongside nodes and the entry point when a mutation is rolled back. +Tests should verify that a successful healing pass drains the queue and that a +failed mutation leaves its prior contents unchanged. + ## Benchmark dataset recipes The `chutoro-bench-datasets` crate defines the shared recipe surface for From 756c7f2ef81b5498c9acf9ea02708c9fa23b87ab Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 24 Aug 2026 15:17:01 +0200 Subject: [PATCH 3/3] Harden localized test healing (#47) Restore the touched-node queue when a deletion rolls back and keep Kani builds independent of test-only tracking APIs. Exercise `CpuHnsw::heal_for_test` directly for insertion, deletion, and reachability repair, including its locality and queue-draining contracts. --- chutoro-core/src/hnsw/cpu/unit_tests.rs | 173 +++++++++++++++++- .../src/hnsw/graph/test_helpers/mod.rs | 2 + .../src/hnsw/graph/test_helpers/tests.rs | 7 + chutoro-core/src/hnsw/insert/test_helpers.rs | 2 + 4 files changed, 183 insertions(+), 1 deletion(-) diff --git a/chutoro-core/src/hnsw/cpu/unit_tests.rs b/chutoro-core/src/hnsw/cpu/unit_tests.rs index 8ff919aa..50aad57b 100644 --- a/chutoro-core/src/hnsw/cpu/unit_tests.rs +++ b/chutoro-core/src/hnsw/cpu/unit_tests.rs @@ -1,7 +1,12 @@ //! Unit tests for the CPU HNSW index. use super::*; -use crate::{MetricDescriptor, datasource::DataSource, error::DataSourceError, hnsw::HnswParams}; +use crate::{ + MetricDescriptor, + datasource::DataSource, + error::DataSourceError, + hnsw::{HnswParams, graph::NodeContext, insert::test_helpers::add_edge_if_missing}, +}; use std::{ sync::{ Arc, @@ -48,6 +53,172 @@ fn insert_waits_for_mutex() { assert!(finished.load(AtomicOrdering::SeqCst)); } +#[test] +fn heal_for_test_repairs_inserted_edges_without_sweeping_unrelated_edges() { + let params = HnswParams::new(2, 4).expect("params").with_rng_seed(41); + let index = CpuHnsw::with_capacity(params, 4).expect("index"); + let source = TestSource::new(vec![0.0, 1.0, 2.0, 3.0]); + + index.insert(0, &source).expect("insert entry"); + index.heal_for_test(); + index.insert(1, &source).expect("insert neighbour"); + + index + .write_graph(|graph| { + let touched = graph.take_touched_nodes(); + assert!( + touched.contains(&(1, 0)), + "insertion must record the new node for localized healing" + ); + graph.record_touched_nodes(touched); + + graph + .attach_node(NodeContext { + node: 2, + level: 0, + sequence: 2, + }) + .expect("attach unrelated node"); + graph + .attach_node(NodeContext { + node: 3, + level: 0, + sequence: 3, + }) + .expect("attach unrelated target"); + graph + .node_mut(0) + .expect("entry must exist") + .neighbours_mut(0) + .retain(|neighbour| *neighbour != 1); + add_edge_if_missing(graph, 0, 2, 0); + add_edge_if_missing(graph, 2, 0, 0); + add_edge_if_missing(graph, 0, 3, 0); + add_edge_if_missing(graph, 3, 0, 0); + add_edge_if_missing(graph, 2, 3, 0); + Ok(()) + }) + .expect("prepare asymmetric edges"); + + index.heal_for_test(); + + index.inspect_graph(|graph| { + let node0 = graph.node(0).expect("entry must remain"); + let node1 = graph.node(1).expect("inserted node must remain"); + let node2 = graph.node(2).expect("unrelated node must remain"); + let node3 = graph.node(3).expect("unrelated target must remain"); + assert!( + node0.neighbours(0).contains(&1), + "healing must restore the reciprocal edge for the touched insertion" + ); + assert!(node1.neighbours(0).contains(&0)); + assert!( + node2.neighbours(0).contains(&3), + "the unrelated asymmetric edge is preserved for the locality check" + ); + assert!( + !node3.neighbours(0).contains(&2), + "healing must not sweep an untouched asymmetric edge" + ); + }); + let touched = index + .write_graph(|graph| Ok(graph.take_touched_nodes())) + .expect("read healing queue"); + assert!(touched.is_empty(), "healing must drain the insertion queue"); +} + +#[test] +fn heal_for_test_drains_tracking_created_by_deletion() { + let params = HnswParams::new(2, 4).expect("params").with_rng_seed(43); + let mut index = CpuHnsw::with_capacity(params, 3).expect("index"); + let source = TestSource::new(vec![0.0, 1.0, 2.0]); + + for node in 0..source.len() { + index.insert(node, &source).expect("insert node"); + } + index + .write_graph(|graph| Ok(graph.take_touched_nodes())) + .expect("clear insertion tracking"); + + assert!( + index.delete_node_for_test(1).expect("delete node"), + "existing node should be deleted" + ); + let touched = index + .write_graph(|graph| Ok(graph.take_touched_nodes())) + .expect("read deletion tracking"); + assert!( + !touched.is_empty(), + "deletion must record changed adjacency lists for healing" + ); + index + .write_graph(|graph| { + graph.record_touched_nodes(touched); + Ok(()) + }) + .expect("restore deletion tracking"); + + index.heal_for_test(); + + assert!(index.inspect_graph(|graph| graph.node(1).is_none())); + let touched = index + .write_graph(|graph| Ok(graph.take_touched_nodes())) + .expect("read healing queue"); + assert!(touched.is_empty(), "healing must drain the deletion queue"); +} + +#[test] +fn heal_for_test_repairs_reachability_and_drains_its_tracking() { + let params = HnswParams::new(2, 4).expect("params").with_rng_seed(47); + let index = CpuHnsw::with_capacity(params, 3).expect("index"); + + index + .write_graph(|graph| { + graph.insert_first(NodeContext { + node: 0, + level: 0, + sequence: 0, + })?; + graph.attach_node(NodeContext { + node: 1, + level: 0, + sequence: 1, + })?; + graph.attach_node(NodeContext { + node: 2, + level: 0, + sequence: 2, + })?; + add_edge_if_missing(graph, 0, 1, 0); + add_edge_if_missing(graph, 1, 0, 0); + let _ = graph.take_touched_nodes(); + Ok(()) + }) + .expect("prepare disconnected graph"); + + index.heal_for_test(); + + index.inspect_graph(|graph| { + let entry = graph.node(0).expect("entry must remain"); + let repaired = graph.node(2).expect("isolated node must remain"); + assert!( + entry.neighbours(0).contains(&2), + "reachability repair must link the isolated node from the entry" + ); + assert!( + repaired.neighbours(0).contains(&0), + "reachability repair must create a reciprocal link" + ); + }); + let touched = index + .write_graph(|graph| Ok(graph.take_touched_nodes())) + .expect("read healing queue"); + assert!( + touched.is_empty(), + "healing must consume tracking created by reachability repair" + ); +} + #[derive(Clone)] struct TestSource { data: Vec, diff --git a/chutoro-core/src/hnsw/graph/test_helpers/mod.rs b/chutoro-core/src/hnsw/graph/test_helpers/mod.rs index 1204c699..f242b7f9 100644 --- a/chutoro-core/src/hnsw/graph/test_helpers/mod.rs +++ b/chutoro-core/src/hnsw/graph/test_helpers/mod.rs @@ -44,6 +44,7 @@ impl Graph { let snapshot_nodes = self.nodes.clone(); let snapshot_entry = self.entry; + let snapshot_touched = self.touched.clone(); let removed_neighbours = collect_neighbour_layers(existing); let Some(_taken) = self.nodes.get_mut(node).and_then(Option::take) else { @@ -61,6 +62,7 @@ impl Graph { if let Err(err) = self.ensure_reachability() { self.nodes = snapshot_nodes; self.entry = snapshot_entry; + self.touched = snapshot_touched; return Err(err); } diff --git a/chutoro-core/src/hnsw/graph/test_helpers/tests.rs b/chutoro-core/src/hnsw/graph/test_helpers/tests.rs index a186f03f..7967357c 100644 --- a/chutoro-core/src/hnsw/graph/test_helpers/tests.rs +++ b/chutoro-core/src/hnsw/graph/test_helpers/tests.rs @@ -137,6 +137,8 @@ fn delete_node_reverts_when_it_would_disconnect_graph(restricted_params: HnswPar graph.try_add_bidirectional_edge(0, 3, 0); graph.try_add_bidirectional_edge(1, 4, 0); graph.try_add_bidirectional_edge(2, 4, 0); + let _ = graph.take_touched_nodes(); + graph.record_touched_nodes([(4, 0)]); let result = graph.delete_node(0); @@ -159,4 +161,9 @@ fn delete_node_reverts_when_it_would_disconnect_graph(restricted_params: HnswPar Some(0), "entry point must roll back on failure" ); + assert_eq!( + graph.take_touched_nodes(), + vec![(4, 0)], + "failed deletion must restore the pre-mutation healing queue", + ); } diff --git a/chutoro-core/src/hnsw/insert/test_helpers.rs b/chutoro-core/src/hnsw/insert/test_helpers.rs index 51ea08a8..df264ff1 100644 --- a/chutoro-core/src/hnsw/insert/test_helpers.rs +++ b/chutoro-core/src/hnsw/insert/test_helpers.rs @@ -101,6 +101,7 @@ impl<'graph> TestHelpers<'graph> { }; let mut healer = ConnectivityHealer::new(self.graph); if healer.link_new_node(&ctx, node_id) { + #[cfg(test)] self.graph.record_touched_nodes([(origin, 0), (node_id, 0)]); return true; } @@ -114,6 +115,7 @@ impl<'graph> TestHelpers<'graph> { }; let mut healer = ConnectivityHealer::new(self.graph); if healer.link_new_node(&ctx, node_id) { + #[cfg(test)] self.graph.record_touched_nodes([(origin, 0), (node_id, 0)]); return true; }