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 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..bac094fd3 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!( + (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!( + (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). + 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!( + (cfg.benna_fast_rate - 0.5_f32).abs() < f32::EPSILON, + "benna_fast_rate default must match GraphStore::new default of 0.5" + ); + assert!( + (cfg.benna_slow_rate - 0.05_f32).abs() < f32::EPSILON, + "benna_slow_rate default must match GraphStore::new default of 0.05" + ); + } }