Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
16 changes: 15 additions & 1 deletion crates/ruvector-mincut/src/graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,11 +258,25 @@ impl DynamicGraph {
}

/// Get all vertices
///
/// Returned in ascending `VertexId` order. `DashMap`'s default hasher is
/// randomly seeded per instance, so its iteration order is *not* stable
/// across process runs even for byte-identical insertion sequences; every
/// caller that uses this list to drive an algorithm with order-dependent
/// tie-breaking (seed selection, bitmask-to-vertex assignment, etc.) needs
/// a canonical order to be reproducible. Sorting here, once, at the graph
/// boundary is cheaper than auditing every downstream consumer.
pub fn vertices(&self) -> Vec<VertexId> {
self.adjacency.iter().map(|entry| *entry.key()).collect()
let mut vertices: Vec<VertexId> = self.adjacency.iter().map(|entry| *entry.key()).collect();
vertices.sort_unstable();
vertices
}

/// Get all edges
///
/// Order is unspecified (`DashMap` iteration). Deliberately left unsorted:
/// this is called per boundary edge in `LocalKCut::check_cut` (#942), and
/// determinism of `partition()` comes from [`Self::vertices`] ordering.
pub fn edges(&self) -> Vec<Edge> {
self.edges.iter().map(|entry| *entry.value()).collect()
}
Expand Down
31 changes: 24 additions & 7 deletions crates/ruvector-mincut/src/instance/bounded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,15 +229,22 @@ impl BoundedInstance {
}
}

// If we have boundary vertices, use them; otherwise fall back to all vertices
if boundary_vertices.is_empty() {
// If we have boundary vertices, use them; otherwise fall back to all vertices.
// Sorted for the same reason as `brute_force_min_cut`: `search_for_cuts`
// below returns on the *first* seed whose search succeeds, so an
// unstable (HashSet-derived) seed order changes which witness is found.
let mut seeds: Vec<VertexId> = if boundary_vertices.is_empty() {
self.vertices.iter().copied().collect()
} else {
boundary_vertices.into_iter().collect()
}
};
seeds.sort_unstable();
seeds
} else {
// No hierarchy - use all vertices
self.vertices.iter().copied().collect()
// No hierarchy - use all vertices, sorted (see comment above).
let mut seeds: Vec<VertexId> = self.vertices.iter().copied().collect();
seeds.sort_unstable();
seeds
};

// Try different budgets within our range
Expand Down Expand Up @@ -296,7 +303,15 @@ impl BoundedInstance {
return None;
}

let vertex_vec: Vec<_> = self.vertices.iter().copied().collect();
// Sorted, not raw HashSet iteration order: `HashSet<VertexId>` uses a
// randomly-seeded hasher per instance, so its iteration order is not
// stable across process runs even for identical insertions. The mask
// loop below assigns bit positions by index into this vector and
// keeps the *first* subset that matches `min_cut` on ties (line
// below: `boundary < min_cut`, strict), so an unstable vertex order
// silently changes which of several equal-value cuts gets returned.
let mut vertex_vec: Vec<_> = self.vertices.iter().copied().collect();
vertex_vec.sort_unstable();
let n = vertex_vec.len();

if n <= 1 {
Expand Down Expand Up @@ -334,7 +349,9 @@ impl BoundedInstance {
}

let membership: RoaringBitmap = best_set.iter().map(|&v| v as u32).collect();
let seed = *best_set.iter().next().unwrap();
// Deterministic seed choice (min, not `HashSet::iter().next()`) so the
// reported witness is reproducible for a fixed `best_set`.
let seed = *best_set.iter().min().unwrap();
let witness = WitnessHandle::new(seed, membership, min_cut);

Some((min_cut, witness))
Expand Down
11 changes: 9 additions & 2 deletions crates/ruvector-mincut/src/instance/witness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,15 @@ impl WitnessHandle {
///
/// # Note
///
/// This method assumes vertices are numbered 0..max_vertex. For sparse
/// graphs, V \ U may contain vertex IDs that don't exist in the graph.
/// This method assumes vertices are numbered `0..=max(U)` — i.e. that the
/// cut side `U` itself contains the graph's highest-numbered vertex. For
/// sparse graphs this is frequently false (a cut side that happens not to
/// include the graph's max-ID vertex), and `V \ U` will silently come
/// back truncated or empty instead of containing the true complement.
/// Callers that need a correct complement over the *actual* graph should
/// not use this method; instead iterate the graph's real vertex list and
/// partition it with [`WitnessHandle::contains`], as
/// `RuVectorGraphAnalyzer::partition` does.
///
/// # Examples
///
Expand Down
21 changes: 19 additions & 2 deletions crates/ruvector-mincut/src/integration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,25 @@ impl RuVectorGraphAnalyzer {
Some((Vec::new(), Vec::new()))
}
MinCutResult::Value { witness, .. } => {
let (side_a, side_b) = witness.materialize_partition();
let partition = (side_a.into_iter().collect(), side_b.into_iter().collect());
// Deliberately not `witness.materialize_partition()`: it infers
// the graph's vertex range from `max(U)` (the cut side's own
// membership), not the graph's actual vertex set. Whenever U
// omits the graph's highest-numbered vertex (the common case —
// e.g. a 9-vertex cluster {0..=8} cut out of a 19-vertex graph
// whose other side is {9..=18}), that under-estimates the
// range and V\U comes back truncated or entirely empty. Build
// both sides from the real vertex list instead: O(|V|), same
// as `materialize_partition`, but correct for any V\U.
let mut side_a = Vec::new();
let mut side_b = Vec::new();
for v in self.graph.vertices() {
if witness.contains(v) {
side_a.push(v);
} else {
side_b.push(v);
}
}
let partition = (side_a, side_b);
self.cached_partition = Some(partition.clone());
Some(partition)
}
Expand Down
164 changes: 164 additions & 0 deletions crates/ruvector-mincut/tests/determinism_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
//! Regression tests for run-to-run determinism of `RuVectorGraphAnalyzer::partition()`.
//!
//! Background: the 2026-09-05 nightly research run
//! (`docs/research/nightly/2026-09-05-mincut-gated-forgetting`) measured
//! `RuVectorGraphAnalyzer::partition()` returning an empty/degenerate result
//! in 15/30 repeated calls against a byte-identical 19-vertex graph, and
//! attributed it to "hash-map iteration-order-dependent tie-breaking" without
//! pinpointing the exact source. Root cause: `BoundedInstance` (the
//! `ProperCutInstance` used for small graphs, `<20` vertices) enumerates its
//! candidate cut vertices from a `HashSet<VertexId>`, whose iteration order
//! is not stable across process runs (Rust's default hasher is randomly
//! seeded per `HashSet`/`DashMap` instance, independent of insertion order).
//! That unstable order fed both a strict `<` tie-break in
//! `brute_force_min_cut` and the top-level `DynamicConnectivity` check via
//! `DynamicGraph::vertices()`/`edges()`, which is why the wrapper sometimes
//! reported the graph as disconnected (`MinCutResult::Disconnected`) and
//! `partition()` returned `Some((vec![], vec![]))`.
//!
//! These tests reproduce the exact topology from that probe
//! (`crates/ruvector-agent-memory/examples/mincut_determinism_probe.rs`) and
//! assert the fixed, deterministic behavior.

use ruvector_mincut::RuVectorGraphAnalyzer;

fn normalize3(v: [f64; 3]) -> Vec<f64> {
let n = (v[0] * v[0] + v[1] * v[1] + v[2] * v[2]).sqrt();
vec![v[0] / n, v[1] / n, v[2] / n]
}

fn cosine_sim(a: &[f64], b: &[f64]) -> f64 {
let dot: f64 = a.iter().zip(b).map(|(x, y)| x * y).sum();
let na: f64 = a.iter().map(|x| x * x).sum::<f64>().sqrt();
let nb: f64 = b.iter().map(|x| x * x).sum::<f64>().sqrt();
if na < 1e-9 || nb < 1e-9 {
0.0
} else {
(dot / (na * nb)).clamp(-1.0, 1.0)
}
}

/// Two 9-point clusters (one axis-aligned "plain" point repeated 8x, plus one
/// "gateway" point tilted toward the other cluster) plus a single equidistant
/// bridge point. This is the same topology the 2026-09-05 nightly used to
/// demonstrate non-determinism: it is symmetric across the two clusters, so
/// the minimum cut has ties that a stable algorithm must break the same way
/// every time.
fn two_cluster_bridge_neighbors() -> (usize, Vec<(usize, Vec<(usize, f64)>)>) {
let mut entries: Vec<Vec<f64>> = Vec::new();
for axis in 0..2 {
let plain = if axis == 0 {
[1.0, 0.0, 0.0]
} else {
[0.0, 1.0, 0.0]
};
let gateway = if axis == 0 {
normalize3([1.0, 0.0, 0.5])
} else {
normalize3([0.0, 1.0, 0.5])
};
for _ in 0..8 {
entries.push(plain.to_vec());
}
entries.push(gateway);
}
entries.push(vec![0.0, 0.0, 1.0]);
let n = entries.len();
let bridge_idx = n - 1;

let k = 8usize;
let min_sim = 0.05f64;
let neighbors: Vec<(usize, Vec<(usize, f64)>)> = (0..n)
.map(|i| {
let mut sims: Vec<(usize, f64)> = (0..n)
.filter(|&j| j != i)
.map(|j| (j, cosine_sim(&entries[i], &entries[j])))
.filter(|&(_, s)| s >= min_sim)
.collect();
sims.sort_unstable_by(|a, b| b.1.partial_cmp(&a.1).unwrap());
sims.truncate(k);
let dists = sims
.into_iter()
.map(|(j, s)| (j, (1.0 - s).max(1e-4)))
.collect();
(i, dists)
})
.collect();

(bridge_idx, neighbors)
}

/// `partition()` must never return a degenerate (one side empty) result on a
/// connected graph: that previously happened in 15/30 trials.
#[test]
fn partition_is_never_degenerate_on_connected_graph() {
let (_bridge_idx, neighbors) = two_cluster_bridge_neighbors();

for trial in 0..30 {
let mut analyzer = RuVectorGraphAnalyzer::from_knn(&neighbors);
let (side_a, side_b) = analyzer
.partition()
.unwrap_or_else(|| panic!("trial {trial}: partition() returned None"));
assert!(
!side_a.is_empty() && !side_b.is_empty(),
"trial {trial}: degenerate partition (one side empty) on a connected graph \
(side_a={}, side_b={})",
side_a.len(),
side_b.len(),
);
}
}

/// Repeated `partition()` calls against a byte-identical graph must return
/// the *same* partition every time. This is the direct determinism
/// regression test for the fix in `DynamicGraph::vertices()`/`edges()` and
/// `BoundedInstance::{brute_force_min_cut, search_for_cuts}`.
#[test]
fn partition_is_stable_across_repeated_calls() {
let (_bridge_idx, neighbors) = two_cluster_bridge_neighbors();

let mut first: Option<(Vec<u64>, Vec<u64>)> = None;
for trial in 0..30 {
let mut analyzer = RuVectorGraphAnalyzer::from_knn(&neighbors);
let (mut side_a, mut side_b) = analyzer.partition().expect("partition should succeed");
side_a.sort_unstable();
side_b.sort_unstable();
// Canonicalize which side is "a" vs "b" by the side containing vertex 0.
let canon = if side_a.contains(&0) {
(side_a, side_b)
} else {
(side_b, side_a)
};
match &first {
None => first = Some(canon),
Some(expected) => assert_eq!(
&canon, expected,
"trial {trial}: partition differs from trial 0's result"
),
}
}
}

/// `DynamicGraph::vertices()` must return a canonical (sorted) order
/// regardless of `DashMap`'s internal iteration order. `edges()` is left
/// unsorted on purpose (hot path in `LocalKCut::check_cut`, #942).
#[test]
fn graph_vertices_are_sorted() {
use ruvector_mincut::graph::DynamicGraph;
use std::sync::Arc;

let graph = Arc::new(DynamicGraph::new());
// Insert out of order on purpose.
for &(u, v) in &[(5u64, 1u64), (0, 9), (3, 2), (8, 4), (7, 6)] {
graph.insert_edge(u, v, 1.0).unwrap();
}

let vertices = graph.vertices();
let mut sorted = vertices.clone();
sorted.sort_unstable();
assert_eq!(
vertices, sorted,
"vertices() must be returned in sorted order"
);
assert_eq!(graph.edges().len(), 5);
}
Loading
Loading