From a6e50c347ff0c6256762b6b3d45ff4b07ec77dde Mon Sep 17 00:00:00 2001 From: "Andrei G." Date: Sat, 30 May 2026 03:38:19 +0200 Subject: [PATCH 1/3] fix(memory): propagate configured Benna-Fusi rates through extract_and_store extract_and_store created a bare GraphStore::new(pool) without calling .with_benna_rates(), causing all Benna-Fusi synaptic updates in the primary graph extraction path to use hardcoded defaults (fast=0.5, slow=0.05) instead of the operator-configured values from [memory.graph.spreading_activation]. Thread benna_fast_rate and benna_slow_rate through GraphExtractionConfig and call .with_benna_rates() in extract_and_store and in the backfill path in agent_access_impl.rs. Also apply two related memory-subsystem improvements from the same review pass: - perf(memory): replace Vec::contains with HashSet in HELA BFS hop loop, reducing O(F^2) comparisons to O(1) per edge (#4698) - perf(memory): replace to_lowercase() allocations in is_low_signal_relation with eq_ignore_ascii_case, eliminating ~30 short-string allocs per extraction pass (#4699) Closes #4711, #4698, #4699 --- crates/zeph-agent-persistence/src/graph.rs | 2 + .../zeph-core/src/agent/agent_access_impl.rs | 2 + crates/zeph-memory/src/graph/activation.rs | 5 +- crates/zeph-memory/src/semantic/graph.rs | 116 +++++++++++++++++- 4 files changed, 120 insertions(+), 5 deletions(-) diff --git a/crates/zeph-agent-persistence/src/graph.rs b/crates/zeph-agent-persistence/src/graph.rs index 6fcf24eb5..b389c5ffa 100644 --- a/crates/zeph-agent-persistence/src/graph.rs +++ b/crates/zeph-agent-persistence/src/graph.rs @@ -57,6 +57,8 @@ pub fn build_graph_extraction_config( .write_gate .enabled .then_some(cfg.write_gate.min_edge_relevance), + benna_fast_rate: cfg.spreading_activation.benna_fast_rate, + benna_slow_rate: cfg.spreading_activation.benna_slow_rate, } } diff --git a/crates/zeph-core/src/agent/agent_access_impl.rs b/crates/zeph-core/src/agent/agent_access_impl.rs index 030edf8c4..5e6db03e3 100644 --- a/crates/zeph-core/src/agent/agent_access_impl.rs +++ b/crates/zeph-core/src/agent/agent_access_impl.rs @@ -596,6 +596,8 @@ impl AgentAccess for Agent { .write_gate .enabled .then_some(graph_cfg.write_gate.min_edge_relevance), + benna_fast_rate: graph_cfg.spreading_activation.benna_fast_rate, + benna_slow_rate: graph_cfg.spreading_activation.benna_slow_rate, }; let pool = store.pool().clone(); match extract_and_store( diff --git a/crates/zeph-memory/src/graph/activation.rs b/crates/zeph-memory/src/graph/activation.rs index a11a43a7f..ab935a932 100644 --- a/crates/zeph-memory/src/graph/activation.rs +++ b/crates/zeph-memory/src/graph/activation.rs @@ -13,7 +13,7 @@ //! - Per-hop pruning to enforce `max_activated_nodes` bound (SA-INV-04) //! - MAGMA edge type filtering via `edge_types` parameter -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::sync::OnceLock; use std::time::{Instant, SystemTime, UNIX_EPOCH}; #[allow(unused_imports)] @@ -295,6 +295,7 @@ pub async fn hela_spreading_recall( } let mut next_frontier: Vec = Vec::new(); + let mut next_frontier_set: HashSet = HashSet::new(); for edge in &edges { // Cache by edge id to avoid repeated clones per source in frontier. @@ -324,7 +325,7 @@ pub async fn hela_spreading_recall( || ((new_pw - entry.1).abs() < f32::EPSILON && hop + 1 < entry.0) { *entry = (hop + 1, new_pw, Some(edge.id)); - if !next_frontier.contains(&neighbor) { + if next_frontier_set.insert(neighbor) { next_frontier.push(neighbor); } } diff --git a/crates/zeph-memory/src/semantic/graph.rs b/crates/zeph-memory/src/semantic/graph.rs index a0843d202..f3650f51f 100644 --- a/crates/zeph-memory/src/semantic/graph.rs +++ b/crates/zeph-memory/src/semantic/graph.rs @@ -70,6 +70,14 @@ pub struct GraphExtractionConfig { /// /// `None` disables the gate (default behaviour, always writes). pub write_gate_min_relevance: Option, + /// Benna-Fusi fast-variable learning rate for confidence merges (#4711). + /// + /// Passed to [`crate::graph::GraphStore::with_benna_rates`]. Default: `0.5`. + pub benna_fast_rate: f32, + /// Benna-Fusi slow-variable learning rate for confidence merges (#4711). + /// + /// Passed to [`crate::graph::GraphStore::with_benna_rates`]. Default: `0.05`. + pub benna_slow_rate: f32, } impl Default for GraphExtractionConfig { @@ -95,6 +103,8 @@ impl Default for GraphExtractionConfig { embed_timeout_secs: 5, turn_index: None, write_gate_min_relevance: None, + benna_fast_rate: 0.5, + benna_slow_rate: 0.05, } } } @@ -391,7 +401,8 @@ pub async fn extract_and_store( ); let ctx_refs: Vec<&str> = context_messages.iter().map(String::as_str).collect(); - let store = GraphStore::new(pool); + let store = + GraphStore::new(pool).with_benna_rates(config.benna_fast_rate, config.benna_slow_rate); bump_extraction_count(store.pool()).await?; @@ -510,8 +521,7 @@ fn is_low_signal_relation(relation: &str) -> bool { "involves", "involved", ]; - let lower = relation.to_lowercase(); - LOW_SIGNAL.iter().any(|&s| lower == s) + LOW_SIGNAL.iter().any(|&s| relation.eq_ignore_ascii_case(s)) } /// Insert extracted edges that have both endpoints in `name_to_id`. @@ -1371,4 +1381,104 @@ mod tests { "born_in must NOT be low-signal" ); } + + /// Regression test for #4711: configured Benna-Fusi rates must produce different + /// `confidence_fast`/`confidence_slow` values than the hardcoded defaults. + /// + /// `extract_and_store` builds `GraphStore::new(pool).with_benna_rates(fast, slow)` using + /// `config.benna_fast_rate` / `config.benna_slow_rate`. Before the fix it called + /// `GraphStore::new(pool)` only, so the configured rates were silently ignored. + /// + /// This test calls `GraphStore::insert_edge_typed` directly (bypassing the resolver dedup + /// layer) to exercise the Benna-Fusi UPDATE path with two confidence levels (0.6 → 0.8). + /// + /// Math: + /// default (0.5/0.05): fast = 0.6 + 0.5*(0.8-0.6) = 0.7; slow ≈ 0.605 + /// custom (0.1/0.02): fast = 0.6 + 0.1*(0.8-0.6) = 0.62; slow ≈ 0.6004 + #[tokio::test] + async fn extract_and_store_respects_configured_benna_rates() { + use crate::graph::EdgeType; + + async fn run_two_inserts(fast_rate: f32, slow_rate: f32) -> crate::graph::types::Edge { + let sqlite = crate::store::SqliteStore::new(":memory:").await.unwrap(); + let pool = sqlite.pool().clone(); + let gs = GraphStore::new(pool).with_benna_rates(fast_rate, slow_rate); + + let alice_id = gs + .upsert_entity("Alice", "alice", crate::graph::EntityType::Person, None) + .await + .unwrap(); + let bob_id = gs + .upsert_entity("Bob", "bob", crate::graph::EntityType::Person, None) + .await + .unwrap(); + + // Pass 1: INSERT — seeds confidence_fast = confidence_slow = 0.6. + gs.insert_edge_typed( + alice_id.0, + bob_id.0, + "knows", + "Alice knows Bob", + 0.6, + None, + EdgeType::Semantic, + ) + .await + .unwrap(); + + // Pass 2: UPDATE — triggers Benna-Fusi merge with incoming confidence = 0.8. + gs.insert_edge_typed( + alice_id.0, + bob_id.0, + "knows", + "Alice knows Bob", + 0.8, + None, + EdgeType::Semantic, + ) + .await + .unwrap(); + + let mut edges = gs.edges_exact(alice_id.0, bob_id.0).await.unwrap(); + assert_eq!(edges.len(), 1, "exactly one active edge expected"); + edges.remove(0) + } + + let default_edge = run_two_inserts(0.5, 0.05).await; + let custom_edge = run_two_inserts(0.1, 0.02).await; + + // Different rates → different fast/slow. Before the fix extract_and_store ignored the + // config fields; all edges would have been identical regardless of configured rates. + assert_ne!( + default_edge.confidence_fast, custom_edge.confidence_fast, + "confidence_fast must differ between default (0.5) and custom (0.1) benna_fast_rate (#4711)" + ); + assert_ne!( + default_edge.confidence_slow, custom_edge.confidence_slow, + "confidence_slow must differ between default (0.05) and custom (0.02) benna_slow_rate (#4711)" + ); + // Higher fast_rate → fast variable grows more aggressively: 0.7 (default) > 0.62 (custom). + assert!( + default_edge.confidence_fast > custom_edge.confidence_fast, + "higher benna_fast_rate must produce a larger confidence_fast after merge" + ); + } + + /// Verify `GraphExtractionConfig` default benna rates match `GraphStore::new` defaults. + /// + /// If either default is changed in one place but not the other, behavior silently diverges + /// between paths that call `with_benna_rates` and paths that don't. + #[test] + fn graph_extraction_config_benna_defaults_match_graph_store_defaults() { + let cfg = GraphExtractionConfig::default(); + // GraphStore::new uses 0.5 / 0.05 — these must stay in sync. + assert_eq!( + cfg.benna_fast_rate, 0.5, + "benna_fast_rate default must match GraphStore::new default of 0.5" + ); + assert_eq!( + cfg.benna_slow_rate, 0.05, + "benna_slow_rate default must match GraphStore::new default of 0.05" + ); + } } From 499045eb3d8ae5450a7abe58882b6e962a756a98 Mon Sep 17 00:00:00 2001 From: "Andrei G." Date: Sat, 30 May 2026 03:39:21 +0200 Subject: [PATCH 2/3] chore: update CHANGELOG for #4711, #4698, #4699 fixes --- CHANGELOG.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18bb356e8..fa63e3151 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,21 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - `zeph-tools`: `apply_ipi_filter` tracing span now correctly covers the `filter_async().await` call using `#[tracing::instrument]`. Previously a manual `span.enter()` was called after the await point, making the IPI filtering invisible in traces. Closes #4712. +- `zeph-memory`: `extract_and_store` now correctly propagates `benna_fast_rate` and + `benna_slow_rate` from `[memory.graph.spreading_activation]` config to the `GraphStore` + created during graph extraction. Previously a bare `GraphStore::new(pool)` was used, silently + discarding operator-configured Benna-Fusi rates and falling back to hardcoded defaults + (fast=0.5, slow=0.05). Both rates are now threaded through `GraphExtractionConfig` and applied + at all extraction callsites, including the backfill path in `agent_access_impl.rs`. Closes #4711. + +### Performance + +- `zeph-memory`: HELA spreading-activation BFS hop loop now uses a `HashSet` for O(1) + frontier membership checks instead of `Vec::contains()` (O(F) per edge, O(F²) overall). + The `Vec` is preserved for BFS traversal order. Closes #4698. +- `zeph-memory`: `is_low_signal_relation` replaced `to_lowercase() == "..."` comparisons with + `eq_ignore_ascii_case("...")`, eliminating ~30 short-string heap allocations per extraction + pass. Closes #4699. ### Added From 7f196e96eb17ef859689c80df62fc4641f134fda Mon Sep 17 00:00:00 2001 From: "Andrei G." Date: Sat, 30 May 2026 03:49:56 +0200 Subject: [PATCH 3/3] test(memory): fix clippy::float-cmp in benna rate regression tests Replace assert_ne!/assert_eq! on f32 values with epsilon-based assert!((a - b).abs() > f32::EPSILON) comparisons. CI runs clippy with --all-targets which includes test code, and -D clippy::float-cmp is implied by -D warnings. --- crates/zeph-memory/src/semantic/graph.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/zeph-memory/src/semantic/graph.rs b/crates/zeph-memory/src/semantic/graph.rs index f3650f51f..bac094fd3 100644 --- a/crates/zeph-memory/src/semantic/graph.rs +++ b/crates/zeph-memory/src/semantic/graph.rs @@ -1449,12 +1449,12 @@ mod tests { // Different rates → different fast/slow. Before the fix extract_and_store ignored the // config fields; all edges would have been identical regardless of configured rates. - assert_ne!( - default_edge.confidence_fast, custom_edge.confidence_fast, + assert!( + (default_edge.confidence_fast - custom_edge.confidence_fast).abs() > f32::EPSILON, "confidence_fast must differ between default (0.5) and custom (0.1) benna_fast_rate (#4711)" ); - assert_ne!( - default_edge.confidence_slow, custom_edge.confidence_slow, + assert!( + (default_edge.confidence_slow - custom_edge.confidence_slow).abs() > f32::EPSILON, "confidence_slow must differ between default (0.05) and custom (0.02) benna_slow_rate (#4711)" ); // Higher fast_rate → fast variable grows more aggressively: 0.7 (default) > 0.62 (custom). @@ -1472,12 +1472,12 @@ mod tests { fn graph_extraction_config_benna_defaults_match_graph_store_defaults() { let cfg = GraphExtractionConfig::default(); // GraphStore::new uses 0.5 / 0.05 — these must stay in sync. - assert_eq!( - cfg.benna_fast_rate, 0.5, + assert!( + (cfg.benna_fast_rate - 0.5_f32).abs() < f32::EPSILON, "benna_fast_rate default must match GraphStore::new default of 0.5" ); - assert_eq!( - cfg.benna_slow_rate, 0.05, + assert!( + (cfg.benna_slow_rate - 0.05_f32).abs() < f32::EPSILON, "benna_slow_rate default must match GraphStore::new default of 0.05" ); }