From 8ea250386c58f5e3fb73dfb00da00e73b24997e4 Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Thu, 21 May 2026 21:58:45 +0200 Subject: [PATCH 01/10] fix(memory): tag vector_chunks with embedding model_signature + dim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Doc-store vectors were stored as raw blobs with no record of which embedding model produced them. After an embedding-model change, cosine recall silently returned 0 (dimension change) or garbage (same dim, different model) instead of excluding stale vectors — unlike the model-aware memory-tree and STM segment_embeddings paths. - add model_signature + dim columns to vector_chunks (CREATE + idempotent ALTER migration) - stamp the active embedder signature + dim on every chunk write - recall excludes chunks whose signature != the active model; dimension guard skips legacy untagged rows instead of scoring them 0 - tests: write tagging, cross-model exclusion, dim-guard for legacy rows Co-Authored-By: Claude Opus 4.7 (1M context) --- .../memory/store/unified/documents.rs | 21 ++- src/openhuman/memory/store/unified/init.rs | 19 ++ src/openhuman/memory/store/unified/query.rs | 22 ++- .../memory/store/unified/query_tests.rs | 165 ++++++++++++++++++ 4 files changed, 217 insertions(+), 10 deletions(-) diff --git a/src/openhuman/memory/store/unified/documents.rs b/src/openhuman/memory/store/unified/documents.rs index 5891eaa49c..ced96762f4 100644 --- a/src/openhuman/memory/store/unified/documents.rs +++ b/src/openhuman/memory/store/unified/documents.rs @@ -155,18 +155,19 @@ impl UnifiedMemory { let chunks = Self::chunk_document_content(&input.content, 225); for (idx, chunk) in chunks.iter().enumerate() { - let embedding = self - .embedder - .embed_one(chunk) - .await - .ok() - .map(|v| Self::vec_to_bytes(&v)); + // Embed the chunk, capturing the model signature + dimension so recall + // can exclude vectors produced by a different embedding model (cross-model + // cosine is meaningless) and guard against dimension mismatches. + let embedded = self.embedder.embed_one(chunk).await.ok(); + let dim = embedded.as_ref().map(|v| v.len() as i64); + let model_signature = embedded.as_ref().map(|_| self.embedder.signature()); + let embedding = embedded.as_ref().map(|v| Self::vec_to_bytes(v)); let chunk_id = format!("{document_id}:{idx}"); let conn = self.conn.lock(); conn.execute( "INSERT OR REPLACE INTO vector_chunks - (namespace, document_id, chunk_id, text, embedding, metadata_json, created_at, updated_at) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)", + (namespace, document_id, chunk_id, text, embedding, metadata_json, created_at, updated_at, model_signature, dim) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10)", params![ namespace, document_id, @@ -175,7 +176,9 @@ impl UnifiedMemory { embedding, json!({"lancedb_table": format!("ns_{namespace}"), "chunk_index": idx}).to_string(), now, - now + now, + model_signature, + dim ], ) .map_err(|e| format!("insert vector chunk: {e}"))?; diff --git a/src/openhuman/memory/store/unified/init.rs b/src/openhuman/memory/store/unified/init.rs index 134a93c3c3..ba4c173cac 100644 --- a/src/openhuman/memory/store/unified/init.rs +++ b/src/openhuman/memory/store/unified/init.rs @@ -112,11 +112,30 @@ impl UnifiedMemory { metadata_json TEXT NOT NULL, created_at REAL NOT NULL, updated_at REAL NOT NULL, + model_signature TEXT, + dim INTEGER, PRIMARY KEY(namespace, chunk_id) ); CREATE INDEX IF NOT EXISTS idx_vector_chunks_ns_doc ON vector_chunks(namespace, document_id);", )?; + // Tag vector_chunks with the embedding model that produced each vector + // on existing databases (idempotent). Fresh installs get these from the + // CREATE TABLE above; older DBs need the ALTERs so recall can exclude + // vectors generated by a different embedding model (cross-model cosine is + // garbage) and skip dimension mismatches instead of silently scoring 0. + for sql in [ + "ALTER TABLE vector_chunks ADD COLUMN model_signature TEXT", + "ALTER TABLE vector_chunks ADD COLUMN dim INTEGER", + ] { + match conn.execute(sql, []) { + Ok(_) => tracing::debug!("[vector_chunks:init] applied: {sql}"), + Err(e) => { + tracing::trace!("[vector_chunks:init] skipped (probably already exists): {e}") + } + } + } + // Create FTS5 episodic tables (episodic_log, episodic_fts, and their // triggers) so the Archivist can call episodic_insert immediately after // the store is initialised. diff --git a/src/openhuman/memory/store/unified/query.rs b/src/openhuman/memory/store/unified/query.rs index ba010ba86c..9f9af8cf4e 100644 --- a/src/openhuman/memory/store/unified/query.rs +++ b/src/openhuman/memory/store/unified/query.rs @@ -39,6 +39,10 @@ struct StoredChunk { text: String, embedding: Option>, updated_at: f64, + /// Signature of the embedding model that produced `embedding`. `None` for + /// rows written before model tagging was introduced. Used to exclude + /// cross-model vectors from cosine scoring. + model_signature: Option, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -506,7 +510,7 @@ impl UnifiedMemory { let conn = self.conn.lock(); let mut stmt = conn .prepare( - "SELECT document_id, chunk_id, text, embedding, updated_at + "SELECT document_id, chunk_id, text, embedding, updated_at, model_signature FROM vector_chunks WHERE namespace = ?1", ) @@ -526,6 +530,7 @@ impl UnifiedMemory { text: row.get(2).map_err(|e| e.to_string())?, embedding: embedding_blob.as_deref().map(Self::bytes_to_vec), updated_at: row.get(4).map_err(|e| e.to_string())?, + model_signature: row.get(5).map_err(|e| e.to_string())?, }); } Ok(chunks) @@ -544,11 +549,26 @@ impl UnifiedMemory { .embed_one(query) .await .map_err(|e| format!("embedding query: {e}"))?; + let active_signature = self.embedder.signature(); let mut scores = HashMap::new(); for chunk in chunks { let Some(embedding) = chunk.embedding.as_ref() else { continue; }; + // Skip vectors produced by a different embedding model — cosine across + // two embedding spaces is meaningless. Rows with no signature (written + // before model tagging) fall through to the dimension guard below. + if let Some(sig) = chunk.model_signature.as_deref() { + if sig != active_signature { + continue; + } + } + // Dimension guard: a model swap that changed dimensionality leaves + // legacy/untagged vectors at the old length; skip them rather than + // letting cosine_similarity silently return 0. + if embedding.len() != query_embedding.len() { + continue; + } let similarity = Self::cosine_similarity(&query_embedding, embedding); let entry = scores .entry(chunk.document_id.clone()) diff --git a/src/openhuman/memory/store/unified/query_tests.rs b/src/openhuman/memory/store/unified/query_tests.rs index 3f3b8e7af9..65a02bf897 100644 --- a/src/openhuman/memory/store/unified/query_tests.rs +++ b/src/openhuman/memory/store/unified/query_tests.rs @@ -422,3 +422,168 @@ async fn format_context_text_includes_entity_types() { context.context_text ); } + +// ── vector_chunks model-signature guard (embedding model-swap safety) ───────── + +use async_trait::async_trait; + +use crate::openhuman::embeddings::EmbeddingProvider; + +/// Embedder stub that returns a fixed vector for any text, with a controllable +/// name + dimension so tests can produce distinct embedding signatures and +/// dimensionalities. +struct StubEmbedder { + name: &'static str, + vector: Vec, +} + +#[async_trait] +impl EmbeddingProvider for StubEmbedder { + fn name(&self) -> &str { + self.name + } + fn model_id(&self) -> &str { + self.name + } + fn dimensions(&self) -> usize { + self.vector.len() + } + async fn embed(&self, texts: &[&str]) -> anyhow::Result>> { + Ok(texts.iter().map(|_| self.vector.clone()).collect()) + } +} + +fn pref_doc(key: &str, content: &str) -> NamespaceDocumentInput { + NamespaceDocumentInput { + namespace: "user_pref".to_string(), + key: key.to_string(), + title: key.to_string(), + content: content.to_string(), + source_type: "pref".to_string(), + priority: "medium".to_string(), + tags: vec![], + metadata: json!({}), + category: "core".to_string(), + session_id: None, + document_id: None, + } +} + +#[tokio::test] +async fn upsert_tags_vector_chunks_with_signature_and_dim() { + let tmp = TempDir::new().unwrap(); + let embedder = Arc::new(StubEmbedder { + name: "stub-a", + vector: vec![1.0, 0.0, 0.0], + }); + let memory = UnifiedMemory::new(tmp.path(), embedder.clone(), None).unwrap(); + + memory + .upsert_document(pref_doc("reply_language", "Reply in British English.")) + .await + .unwrap(); + + // The stored chunk carries the active model's signature. + let chunks = memory.load_chunks_for_scope("user_pref").await.unwrap(); + assert_eq!(chunks.len(), 1, "expected exactly one chunk for the doc"); + assert_eq!( + chunks[0].model_signature.as_deref(), + Some(embedder.signature().as_str()), + "chunk should be tagged with the embedder signature" + ); + + // The `dim` column reflects the embedding dimensionality. + let dim: Option = memory + .conn + .lock() + .query_row( + "SELECT dim FROM vector_chunks WHERE namespace = 'user_pref' LIMIT 1", + [], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(dim, Some(3)); +} + +#[tokio::test] +async fn vector_recall_excludes_other_model_signature() { + let tmp = TempDir::new().unwrap(); + + // Write under model A. + let emb_a = Arc::new(StubEmbedder { + name: "model-a", + vector: vec![1.0, 0.0, 0.0], + }); + { + let memory = UnifiedMemory::new(tmp.path(), emb_a.clone(), None).unwrap(); + memory + .upsert_document(pref_doc("p1", "formal tone for emails to my manager")) + .await + .unwrap(); + + // Same model → the vector is scored. + let chunks = memory.load_chunks_for_scope("user_pref").await.unwrap(); + let scores = memory + .query_vector_scores_from_chunks(&chunks, "email tone") + .await + .unwrap(); + assert!(!scores.is_empty(), "same-signature vectors must be scored"); + } + + // Reopen the same DB under a DIFFERENT model (swap), same dim + vector. + let emb_b = Arc::new(StubEmbedder { + name: "model-b", + vector: vec![1.0, 0.0, 0.0], + }); + let memory_b = UnifiedMemory::new(tmp.path(), emb_b, None).unwrap(); + let chunks = memory_b.load_chunks_for_scope("user_pref").await.unwrap(); + assert_eq!(chunks.len(), 1, "the chunk persists across reopen"); + let scores = memory_b + .query_vector_scores_from_chunks(&chunks, "email tone") + .await + .unwrap(); + assert!( + scores.is_empty(), + "vectors from a different embedding model must be excluded, not compared as garbage" + ); +} + +#[tokio::test] +async fn vector_recall_skips_dimension_mismatch_for_untagged_rows() { + let tmp = TempDir::new().unwrap(); + // Active model produces 4-dim vectors. + let emb = Arc::new(StubEmbedder { + name: "model-a", + vector: vec![1.0, 0.0, 0.0, 0.0], + }); + let memory = UnifiedMemory::new(tmp.path(), emb, None).unwrap(); + + // Insert a legacy chunk: NULL signature, 2-dim vector (a pre-tagging row left + // behind by a dimension-changing model swap). + let legacy_vec = UnifiedMemory::vec_to_bytes(&[1.0_f32, 0.0]); + memory + .conn + .lock() + .execute( + "INSERT INTO vector_chunks + (namespace, document_id, chunk_id, text, embedding, metadata_json, created_at, updated_at, model_signature, dim) + VALUES ('user_pref','legacy','legacy:0','old pref',?1,'{}',0,0,NULL,2)", + rusqlite::params![legacy_vec], + ) + .unwrap(); + + let chunks = memory.load_chunks_for_scope("user_pref").await.unwrap(); + assert_eq!(chunks.len(), 1); + assert!( + chunks[0].model_signature.is_none(), + "legacy row should have no signature" + ); + let scores = memory + .query_vector_scores_from_chunks(&chunks, "old pref") + .await + .unwrap(); + assert!( + scores.is_empty(), + "dimension-mismatched legacy vectors must be skipped, not scored 0" + ); +} From 466b50489cfac105603d98726192bff9fb669f43 Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Thu, 21 May 2026 22:37:08 +0200 Subject: [PATCH 02/10] =?UTF-8?q?feat(memory):=20two-lane=20user=20prefere?= =?UTF-8?q?nces=20=E2=80=94=20save=5Fpreference=20tool=20+=20Lane=20A?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Explicit, deterministic preference capture split by relevance scope: - save_preference(topic, value, category) writes to user_pref_{general,situational}; topic = dedup key (re-saving replaces), and a topic lives in exactly one scope. Written verbatim — bypasses the inference/stability pipeline. - Lane A: general (always-on) prefs render as a '## Your standing preferences' block in the system prompt at thread start (latest-N by recency, bounded). - Namespace constants + the Lane-A read helper centralised in memory::preferences; the explicit-prefs path no longer reads the legacy user_profile pinned namespace. Lane B (per-turn situational recall), contradiction handling, and demoting the inferred user_profile table to a suggestion feed follow in subsequent commits. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/openhuman/agent/harness/session/turn.rs | 67 +---- src/openhuman/learning/prompt_sections.rs | 4 +- src/openhuman/memory/mod.rs | 1 + src/openhuman/memory/preferences.rs | 51 ++++ src/openhuman/tools/impl/agent/mod.rs | 2 + .../tools/impl/agent/save_preference.rs | 266 ++++++++++++++++++ .../tools/impl/agent/save_preference_tests.rs | 178 ++++++++++++ src/openhuman/tools/ops.rs | 4 + 8 files changed, 518 insertions(+), 55 deletions(-) create mode 100644 src/openhuman/memory/preferences.rs create mode 100644 src/openhuman/tools/impl/agent/save_preference.rs create mode 100644 src/openhuman/tools/impl/agent/save_preference_tests.rs diff --git a/src/openhuman/agent/harness/session/turn.rs b/src/openhuman/agent/harness/session/turn.rs index 3040cd63ee..5d6df2236b 100644 --- a/src/openhuman/agent/harness/session/turn.rs +++ b/src/openhuman/agent/harness/session/turn.rs @@ -1493,63 +1493,24 @@ impl Agent { return LearnedContextData::default(); } - // Narrow explicit-preferences path: only fetch pinned user_profile - // entries; skip all inference-derived data. + // Narrow explicit-preferences path (Lane A): inject the latest-N general + // (always-on) preferences written via `save_preference`. Topic-scoped + // (situational) prefs are NOT injected here — they ride the user message + // via per-turn recall (Lane B). The legacy `user_profile` pinned namespace + // is no longer read here; explicit prefs now live in `user_pref_general`. if !self.learning_enabled && self.explicit_preferences_enabled { + let general = crate::openhuman::memory::preferences::load_general_preferences( + &self.memory, + crate::openhuman::memory::preferences::STANDING_PREFS_LIMIT, + ) + .await; tracing::debug!( - "[learning] fetch_learned_context: explicit_preferences_enabled=true, \ - learning_enabled=false — fetching only pinned user_profile entries" - ); - let profile_entries = self - .memory - .list( - Some("user_profile"), - // Core category is used by RememberPreferenceTool for pinned entries. - // We list without category filter so we pick up both Core entries - // (pinned) and any Custom("user_profile") entries from the older - // UserProfileHook code path, keeping this backward-compatible. - None, - None, - ) - .await - .unwrap_or_default(); - - // `.list()` already scopes to the `user_profile` namespace at the - // store layer (via the `Some("user_profile")` argument above). This - // `.filter()` is a defensive guard against any future store-layer - // change that might weaken that scoping — it is not load-bearing - // under the current implementation. - if profile_entries.len() > 50 { - tracing::warn!( - total = profile_entries.len(), - dropped = profile_entries.len() - 50, - "[learning] user_profile pinned preferences exceed prompt cap of 50; \ - {} entries will be dropped from this turn's context", - profile_entries.len() - 50, - ); - } - let user_profile: Vec = profile_entries - .iter() - .filter(|e| { - e.namespace - .as_deref() - .map_or(false, |ns| ns == "user_profile") - }) - .take(50) - .map(|e| sanitize_learned_entry(&e.content)) - .collect(); - - tracing::debug!( - "[learning] fetch_learned_context: fetched {} pinned user_profile entries", - user_profile.len() + "[learning] fetch_learned_context: explicit_preferences_enabled — loaded {} general preference(s) for the system prompt", + general.len() ); - return LearnedContextData { - observations: Vec::new(), - patterns: Vec::new(), - user_profile, - reflections: Vec::new(), - tree_root_summaries: Vec::new(), + user_profile: general, + ..LearnedContextData::default() }; } diff --git a/src/openhuman/learning/prompt_sections.rs b/src/openhuman/learning/prompt_sections.rs index 378732addf..fe97a13043 100644 --- a/src/openhuman/learning/prompt_sections.rs +++ b/src/openhuman/learning/prompt_sections.rs @@ -86,7 +86,7 @@ impl PromptSection for UserProfileSection { return Ok(String::new()); } - let mut out = String::from("## User Profile (Learned)\n\n"); + let mut out = String::from("## Your standing preferences\n\n"); for entry in &ctx.learned.user_profile { out.push_str("- "); out.push_str(entry); @@ -357,7 +357,7 @@ mod tests { .unwrap(); assert_eq!(section.name(), "user_profile"); - assert!(rendered.starts_with("## User Profile (Learned)\n\n")); + assert!(rendered.starts_with("## Your standing preferences\n\n")); assert!(rendered.contains("- Timezone: America/Los_Angeles")); assert!(rendered.contains("- Prefers Rust")); } diff --git a/src/openhuman/memory/mod.rs b/src/openhuman/memory/mod.rs index 5966a4689f..bb44a8cbd8 100644 --- a/src/openhuman/memory/mod.rs +++ b/src/openhuman/memory/mod.rs @@ -10,6 +10,7 @@ pub mod conversations; pub mod global; pub mod ingestion; pub mod ops; +pub mod preferences; pub mod rpc_models; pub mod safety; pub mod schemas; diff --git a/src/openhuman/memory/preferences.rs b/src/openhuman/memory/preferences.rs new file mode 100644 index 0000000000..6485b265bd --- /dev/null +++ b/src/openhuman/memory/preferences.rs @@ -0,0 +1,51 @@ +//! Two-lane explicit user preferences — namespaces + read helpers. +//! +//! Preferences written by the `save_preference` tool live in one of two +//! namespaces depending on their relevance scope: +//! +//! - [`USER_PREF_GENERAL_NAMESPACE`] — always-on; injected into the system +//! prompt at thread start (Lane A). +//! - [`USER_PREF_SITUATIONAL_NAMESPACE`] — topic-scoped; recalled per-turn by +//! semantic similarity to the user's message (Lane B). +//! +//! Keeping the namespace constants and read helpers here (rather than in the +//! tool module) lets the write path, the system-prompt builder, and the +//! per-turn recall path all share one definition. + +use std::sync::Arc; + +use super::Memory; + +/// Always-on preferences — injected into the system prompt every thread. +pub const USER_PREF_GENERAL_NAMESPACE: &str = "user_pref_general"; + +/// Topic-scoped preferences — recalled per query against the user's message. +pub const USER_PREF_SITUATIONAL_NAMESPACE: &str = "user_pref_situational"; + +/// Default cap on general preferences injected into the system prompt. Keeps +/// the always-on block bounded so it can't blow a small model's context window +/// (see the legacy `gpt-4` 8K overflow). +pub const STANDING_PREFS_LIMIT: usize = 10; + +/// Load the latest-`limit` general preferences as plain-language strings, +/// newest-first (by `updated_at`). This is the Lane-A system-prompt block. +/// +/// `list()` returns entries ordered newest-first but with `content` set to the +/// title (= topic key), so the body value is fetched via `get()`. +pub async fn load_general_preferences(memory: &Arc, limit: usize) -> Vec { + let entries = memory + .list(Some(USER_PREF_GENERAL_NAMESPACE), None, None) + .await + .unwrap_or_default(); + + let mut out = Vec::new(); + for entry in entries.into_iter().take(limit) { + if let Ok(Some(full)) = memory.get(USER_PREF_GENERAL_NAMESPACE, &entry.key).await { + let value = full.content.trim(); + if !value.is_empty() { + out.push(value.to_string()); + } + } + } + out +} diff --git a/src/openhuman/tools/impl/agent/mod.rs b/src/openhuman/tools/impl/agent/mod.rs index 52d0b71f51..6e1f1179ab 100644 --- a/src/openhuman/tools/impl/agent/mod.rs +++ b/src/openhuman/tools/impl/agent/mod.rs @@ -7,6 +7,7 @@ mod dispatch; pub(crate) mod onboarding_status; mod plan_exit; pub mod remember_preference; +pub mod save_preference; mod skill_delegation; mod spawn_parallel_agents; mod spawn_subagent; @@ -22,6 +23,7 @@ pub use complete_onboarding::CompleteOnboardingTool; pub use delegate::DelegateTool; pub use plan_exit::{PlanExitTool, PLAN_EXIT_MARKER}; pub use remember_preference::RememberPreferenceTool; +pub use save_preference::SavePreferenceTool; pub use skill_delegation::SkillDelegationTool; pub use spawn_parallel_agents::SpawnParallelAgentsTool; pub use spawn_subagent::SpawnSubagentTool; diff --git a/src/openhuman/tools/impl/agent/save_preference.rs b/src/openhuman/tools/impl/agent/save_preference.rs new file mode 100644 index 0000000000..46fcd2ea98 --- /dev/null +++ b/src/openhuman/tools/impl/agent/save_preference.rs @@ -0,0 +1,266 @@ +//! `save_preference` — explicit two-lane user-preference capture. +//! +//! Splits a free-form preference into one of two relevance scopes: +//! +//! - **`general`** → applies to *every* reply (tone, language, identity, +//! standing habits). Stored in [`USER_PREF_GENERAL_NAMESPACE`] and injected +//! into the system prompt at thread start (Lane A). +//! - **`situational`** → only relevant when its topic comes up. Stored in +//! [`USER_PREF_SITUATIONAL_NAMESPACE`] and recalled per-turn by semantic +//! similarity to the user's message (Lane B). +//! +//! `topic` is a snake_case slug used as the storage key, so re-saving the same +//! topic overwrites the prior value (no duplicates — `ON CONFLICT REPLACE`). A +//! topic lives in exactly one scope: writing it under one namespace clears any +//! prior copy in the other so a re-categorised preference can't linger in both +//! lanes. +//! +//! Unlike the inference pipeline (`user_profile` facets), these are written +//! verbatim and immediately — they bypass the stability detector entirely. + +use std::sync::Arc; + +use async_trait::async_trait; +use serde_json::json; + +use crate::openhuman::memory::{Memory, MemoryCategory}; +use crate::openhuman::security::policy::ToolOperation; +use crate::openhuman::security::SecurityPolicy; +use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolResult}; + +// Namespace constants live in `memory::preferences` so the write path (here), +// the system-prompt builder (Lane A), and per-turn recall (Lane B) all share a +// single definition. +pub use crate::openhuman::memory::preferences::{ + USER_PREF_GENERAL_NAMESPACE, USER_PREF_SITUATIONAL_NAMESPACE, +}; + +/// Relevance scope chosen by the model when saving a preference. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PrefScope { + /// Applies to every reply regardless of topic. + General, + /// Only relevant when its topic relates to the current message. + Situational, +} + +impl PrefScope { + /// Parse the `category` argument (case-insensitive). + pub fn parse(s: &str) -> Option { + match s.trim().to_ascii_lowercase().as_str() { + "general" => Some(Self::General), + "situational" => Some(Self::Situational), + _ => None, + } + } + + /// Storage namespace for this scope. + pub fn namespace(self) -> &'static str { + match self { + Self::General => USER_PREF_GENERAL_NAMESPACE, + Self::Situational => USER_PREF_SITUATIONAL_NAMESPACE, + } + } + + /// The opposite scope's namespace — cleared on write so a topic lives in + /// exactly one lane. + pub fn other_namespace(self) -> &'static str { + match self { + Self::General => USER_PREF_SITUATIONAL_NAMESPACE, + Self::Situational => USER_PREF_GENERAL_NAMESPACE, + } + } + + pub fn as_str(self) -> &'static str { + match self { + Self::General => "general", + Self::Situational => "situational", + } + } +} + +/// Agent tool that saves an explicit user preference into the two-lane store. +pub struct SavePreferenceTool { + memory: Arc, + security: Arc, +} + +impl SavePreferenceTool { + pub fn new(memory: Arc, security: Arc) -> Self { + Self { memory, security } + } +} + +#[async_trait] +impl Tool for SavePreferenceTool { + fn name(&self) -> &str { + "save_preference" + } + + fn description(&self) -> &str { + "Save a user preference so it shapes future replies. Call this when the user states or \ + asks to remember a preference. Choose `category`:\n\ + - \"general\": applies to EVERY reply regardless of topic — tone, language, identity, \ + standing habits (e.g. \"reply in British English\", \"be terse\", \"I'm in IST\", \ + \"I'm vegetarian\"). Present in every conversation.\n\ + - \"situational\": only relevant when its topic comes up (e.g. \"when writing Rust prefer \ + X\", \"be formal in emails to my manager\", \"my AWS account is Y\"). Surfaced only when \ + the user's message relates to it.\n\ + `topic` is a short snake_case slug (e.g. reply_language, email_tone_boss, cuisine); \ + re-saving the same topic overwrites the previous value — no duplicates are created." + } + + fn parameters_schema(&self) -> serde_json::Value { + json!({ + "type": "object", + "required": ["topic", "value", "category"], + "properties": { + "topic": { + "type": "string", + "description": "Short snake_case slug naming what this preference is about, e.g. \ + reply_language, verbosity, cuisine, email_tone_boss. Lowercase \ + letters, digits, and underscores only. Re-saving the same topic \ + replaces the previous value." + }, + "value": { + "type": "string", + "description": "The preference in plain language, e.g. \"Reply in British English \ + spelling and idiom.\"" + }, + "category": { + "type": "string", + "enum": ["general", "situational"], + "description": "general = applies to every reply; situational = only when the \ + topic is relevant to the current message." + } + } + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::Write + } + + async fn execute(&self, args: serde_json::Value) -> anyhow::Result { + tracing::debug!( + "[tool][save_preference] invoked: topic={:?} category={:?} value_len={}", + args.get("topic").and_then(|v| v.as_str()), + args.get("category").and_then(|v| v.as_str()), + args.get("value") + .and_then(|v| v.as_str()) + .map_or(0, str::len), + ); + + // Security gate — Write-level autonomy, mirroring remember_preference. + if let Err(error) = self + .security + .enforce_tool_operation(ToolOperation::Act, "save_preference") + { + tracing::warn!("[tool][save_preference] security gate rejected: {error}"); + return Ok(ToolResult::error(error)); + } + + // Parse category. + let category = match args.get("category").and_then(|v| v.as_str()) { + Some(s) => match PrefScope::parse(s) { + Some(c) => c, + None => { + return Ok(ToolResult::error(format!( + "invalid category {s:?}; must be \"general\" or \"situational\"" + ))); + } + }, + None => { + return Ok(ToolResult::error( + "missing required argument: category".to_string(), + )); + } + }; + + // Parse topic — non-empty snake_case slug (used as the dedup key). + let topic = match args.get("topic").and_then(|v| v.as_str()) { + Some(t) => t.trim(), + None => { + return Ok(ToolResult::error( + "missing required argument: topic".to_string(), + )); + } + }; + if topic.is_empty() { + return Ok(ToolResult::error("topic cannot be empty".to_string())); + } + if !topic + .chars() + .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '_') + { + return Ok(ToolResult::error(format!( + "topic {topic:?} contains invalid characters; use only lowercase letters, digits, \ + and underscores (snake_case)" + ))); + } + + // Parse value — free-form, trimmed. + let value = match args.get("value").and_then(|v| v.as_str()) { + Some(v) => v.trim(), + None => { + return Ok(ToolResult::error( + "missing required argument: value".to_string(), + )); + } + }; + if value.is_empty() { + return Ok(ToolResult::error("value cannot be empty".to_string())); + } + + let namespace = category.namespace(); + + // A topic lives in exactly one scope — clear any prior copy in the other + // namespace so a re-categorised preference doesn't linger in both lanes. + if let Err(e) = self.memory.forget(category.other_namespace(), topic).await { + tracing::debug!( + "[tool][save_preference] clearing other-scope copy failed (non-fatal) ns={} topic={}: {e}", + category.other_namespace(), + topic + ); + } + + tracing::debug!( + "[tool][save_preference] storing namespace={} topic={} category={} value_len={}", + namespace, + topic, + category.as_str(), + value.len() + ); + + match self + .memory + .store(namespace, topic, value, MemoryCategory::Core, None) + .await + { + Ok(()) => { + tracing::info!( + "[tool][save_preference] saved namespace={} topic={} category={}", + namespace, + topic, + category.as_str() + ); + Ok(ToolResult::success(format!( + "Saved {} preference: {topic} = {value}", + category.as_str() + ))) + } + Err(e) => { + tracing::error!( + "[tool][save_preference] failed to store namespace={} topic={}: {e:#}", + namespace, + topic + ); + Ok(ToolResult::error(format!("Failed to save preference: {e}"))) + } + } + } +} + +#[cfg(test)] +#[path = "save_preference_tests.rs"] +mod tests; diff --git a/src/openhuman/tools/impl/agent/save_preference_tests.rs b/src/openhuman/tools/impl/agent/save_preference_tests.rs new file mode 100644 index 0000000000..a2f9c254a4 --- /dev/null +++ b/src/openhuman/tools/impl/agent/save_preference_tests.rs @@ -0,0 +1,178 @@ +//! Tests for the `save_preference` two-lane preference tool. + +use super::*; + +use crate::openhuman::embeddings::NoopEmbedding; +use crate::openhuman::memory::UnifiedMemory; +use crate::openhuman::security::SecurityPolicy; +use serde_json::json; +use tempfile::TempDir; + +fn test_security() -> Arc { + Arc::new(SecurityPolicy::default()) +} + +fn test_mem() -> (TempDir, Arc) { + let tmp = TempDir::new().unwrap(); + let mem = UnifiedMemory::new(tmp.path(), Arc::new(NoopEmbedding), None).unwrap(); + (tmp, Arc::new(mem)) +} + +async fn keys_in(mem: &Arc, namespace: &str) -> Vec { + mem.list(Some(namespace), None, None) + .await + .unwrap() + .into_iter() + .map(|e| e.key) + .collect() +} + +// ── PrefScope ──────────────────────────────────────────────────────────────── + +#[test] +fn pref_scope_parse_case_insensitive() { + assert_eq!(PrefScope::parse("general"), Some(PrefScope::General)); + assert_eq!( + PrefScope::parse("Situational"), + Some(PrefScope::Situational) + ); + assert_eq!( + PrefScope::parse("SITUATIONAL"), + Some(PrefScope::Situational) + ); + assert_eq!(PrefScope::parse("bogus"), None); + assert_eq!(PrefScope::parse(""), None); +} + +#[test] +fn pref_scope_namespace_mapping() { + assert_eq!(PrefScope::General.namespace(), USER_PREF_GENERAL_NAMESPACE); + assert_eq!( + PrefScope::Situational.namespace(), + USER_PREF_SITUATIONAL_NAMESPACE + ); + assert_eq!( + PrefScope::General.other_namespace(), + USER_PREF_SITUATIONAL_NAMESPACE + ); + assert_eq!( + PrefScope::Situational.other_namespace(), + USER_PREF_GENERAL_NAMESPACE + ); +} + +// ── Tool metadata ───────────────────────────────────────────────────────────── + +#[test] +fn tool_name_and_permission() { + let (_tmp, mem) = test_mem(); + let tool = SavePreferenceTool::new(mem, test_security()); + assert_eq!(tool.name(), "save_preference"); + assert_eq!(tool.permission_level(), PermissionLevel::Write); +} + +#[test] +fn schema_has_required_fields() { + let (_tmp, mem) = test_mem(); + let tool = SavePreferenceTool::new(mem, test_security()); + let schema = tool.parameters_schema(); + let required: Vec<&str> = schema["required"] + .as_array() + .unwrap() + .iter() + .filter_map(|v| v.as_str()) + .collect(); + assert!(required.contains(&"topic")); + assert!(required.contains(&"value")); + assert!(required.contains(&"category")); +} + +// ── Argument validation ───────────────────────────────────────────────────────── + +#[tokio::test] +async fn invalid_category_returns_error() { + let (_tmp, mem) = test_mem(); + let tool = SavePreferenceTool::new(mem, test_security()); + let r = tool + .execute(json!({"topic": "x", "value": "y", "category": "bogus"})) + .await + .unwrap(); + assert!(r.is_error); + assert!(r.output().contains("category")); +} + +#[tokio::test] +async fn invalid_topic_chars_returns_error() { + let (_tmp, mem) = test_mem(); + let tool = SavePreferenceTool::new(mem, test_security()); + let r = tool + .execute(json!({"topic": "Bad Topic!", "value": "y", "category": "general"})) + .await + .unwrap(); + assert!(r.is_error); +} + +#[tokio::test] +async fn empty_value_returns_error() { + let (_tmp, mem) = test_mem(); + let tool = SavePreferenceTool::new(mem, test_security()); + let r = tool + .execute(json!({"topic": "topic", "value": " ", "category": "general"})) + .await + .unwrap(); + assert!(r.is_error); +} + +// ── Storage behaviour ───────────────────────────────────────────────────────── + +#[tokio::test] +async fn saves_general_pref_to_general_namespace() { + let (_tmp, mem) = test_mem(); + let tool = SavePreferenceTool::new(mem.clone(), test_security()); + let r = tool + .execute(json!({ + "topic": "reply_language", + "value": "Reply in British English.", + "category": "general" + })) + .await + .unwrap(); + assert!(!r.is_error, "expected success, got: {}", r.output()); + + assert!(keys_in(&mem, USER_PREF_GENERAL_NAMESPACE) + .await + .contains(&"reply_language".to_string())); + assert!(keys_in(&mem, USER_PREF_SITUATIONAL_NAMESPACE) + .await + .is_empty()); +} + +#[tokio::test] +async fn recategorising_moves_pref_between_namespaces() { + let (_tmp, mem) = test_mem(); + let tool = SavePreferenceTool::new(mem.clone(), test_security()); + + // Save as general. + tool.execute(json!({"topic": "tone", "value": "be terse", "category": "general"})) + .await + .unwrap(); + assert!(keys_in(&mem, USER_PREF_GENERAL_NAMESPACE) + .await + .contains(&"tone".to_string())); + + // Re-save the same topic as situational → moves namespaces, no stale copy. + tool.execute( + json!({"topic": "tone", "value": "be terse in code reviews", "category": "situational"}), + ) + .await + .unwrap(); + assert!(keys_in(&mem, USER_PREF_SITUATIONAL_NAMESPACE) + .await + .contains(&"tone".to_string())); + assert!( + !keys_in(&mem, USER_PREF_GENERAL_NAMESPACE) + .await + .contains(&"tone".to_string()), + "the general-scope copy must be cleared when re-categorised" + ); +} diff --git a/src/openhuman/tools/ops.rs b/src/openhuman/tools/ops.rs index 8254ad5c83..7d2248b146 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -159,6 +159,10 @@ pub fn all_tools_with_runtime( memory.clone(), security.clone(), )), + // Two-lane explicit preferences (general → system prompt, situational → + // per-query recall). Written verbatim to user_pref_{general,situational}; + // bypasses the inference/stability pipeline. Always registered. + Box::new(SavePreferenceTool::new(memory.clone(), security.clone())), // WhatsApp data store — read-only agent surface (issue #1341). // The matching `whatsapp_data_ingest` write-path stays internal-only // (registered in `src/core/all.rs::build_internal_only_controllers`) From 45e212ffe3f70d92547e359b980032b20a237760 Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Fri, 22 May 2026 07:57:03 +0200 Subject: [PATCH 03/10] =?UTF-8?q?feat(memory):=20Lane=20B=20=E2=80=94=20pe?= =?UTF-8?q?r-turn=20situational=20preference=20recall?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Topic-scoped (situational) preferences are recalled every turn against the user's message and injected under a '## Relevant preferences for this message' block on the per-turn context (not the frozen system prompt). - Memory::recall_relevant_by_vector gates on the vector-similarity component alone (default-empty for mocks; UnifiedMemory filters query_namespace_hits by score_breakdown.vector_similarity), so an unrelated message surfaces nothing. - Leverages the model-aware vector_chunks embeddings (worktree A): a stale embedding model signature is excluded rather than mis-scored. - turn.rs injects the situational block every turn; general prefs stay in the frozen system prompt (Lane A). - test: relevant query surfaces the matching pref; unrelated query surfaces none. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/openhuman/agent/harness/session/turn.rs | 34 ++++++- src/openhuman/memory/preferences.rs | 30 +++++++ src/openhuman/memory/store/memory_trait.rs | 19 ++++ .../memory/store/unified/query_tests.rs | 89 ++++++++++++++++++- src/openhuman/memory/traits.rs | 21 +++++ 5 files changed, 191 insertions(+), 2 deletions(-) diff --git a/src/openhuman/agent/harness/session/turn.rs b/src/openhuman/agent/harness/session/turn.rs index 5d6df2236b..b204d9ee3d 100644 --- a/src/openhuman/agent/harness/session/turn.rs +++ b/src/openhuman/agent/harness/session/turn.rs @@ -334,7 +334,7 @@ impl Agent { // Gate: `learning.stm_recall_enabled` must be true AND this must // be the first turn (STM is snapshot-frozen at session start). // Failure is non-fatal — bare `context` passes through untouched. - let context = if is_first_turn_for_stm { + let mut context = if is_first_turn_for_stm { // Load config to check the gate. Use a cached load (cheap). let stm_enabled = crate::openhuman::config::rpc::load_config_with_timeout() .await @@ -388,6 +388,38 @@ impl Agent { context }; + // ── Lane B: situational preferences (every turn) ───────────────────── + // Recall topic-scoped preferences semantically relevant to THIS message + // (model-aware embeddings, gated by vector similarity) and inject them + // under a banner. Runs every turn — unlike the first-turn-gated tree/STM + // blocks above — because the query changes per message; it rides the + // per-turn context that's prepended to the user message (no KV-cache + // cost). An unrelated message clears the similarity gate to nothing, so + // no block is injected. + { + let situational = + crate::openhuman::memory::preferences::recall_situational_preferences( + &self.memory, + user_message, + ) + .await; + if !situational.is_empty() { + log::info!( + "[pref_recall] situational block injected: {} item(s)", + situational.len() + ); + context.push_str("## Relevant preferences for this message\n\n"); + for pref in &situational { + context.push_str("- "); + context.push_str(pref.trim()); + context.push('\n'); + } + context.push('\n'); + } else { + log::debug!("[pref_recall] no situational preference relevant to this message"); + } + } + let enriched = if context.is_empty() { log::info!("[agent] no memory context found — using raw user message"); self.last_memory_context = None; diff --git a/src/openhuman/memory/preferences.rs b/src/openhuman/memory/preferences.rs index 6485b265bd..140f0c56dd 100644 --- a/src/openhuman/memory/preferences.rs +++ b/src/openhuman/memory/preferences.rs @@ -49,3 +49,33 @@ pub async fn load_general_preferences(memory: &Arc, limit: usize) -> } out } + +/// Top-K situational preferences to recall per turn (Lane B). +pub const SITUATIONAL_RECALL_LIMIT: usize = 5; + +/// Minimum query↔preference vector similarity for a situational preference to be +/// injected. Below this the current message isn't considered relevant to the +/// preference, so nothing is injected (the "unrelated query → no block" +/// behaviour). Tunable against live data. +pub const SITUATIONAL_MIN_SIMILARITY: f64 = 0.35; + +/// Recall situational preferences semantically relevant to `query` (Lane B). +/// +/// Returns only preferences whose vector similarity to the message clears +/// [`SITUATIONAL_MIN_SIMILARITY`], so an unrelated message yields an empty list +/// (and no injected block). Uses the model-aware embedding recall, so a stale +/// embedding-model signature is excluded rather than mis-scored. +pub async fn recall_situational_preferences(memory: &Arc, query: &str) -> Vec { + if query.trim().is_empty() { + return Vec::new(); + } + memory + .recall_relevant_by_vector( + USER_PREF_SITUATIONAL_NAMESPACE, + query, + SITUATIONAL_RECALL_LIMIT, + SITUATIONAL_MIN_SIMILARITY, + ) + .await + .unwrap_or_default() +} diff --git a/src/openhuman/memory/store/memory_trait.rs b/src/openhuman/memory/store/memory_trait.rs index 97f313f6a3..28cb4a9cbb 100644 --- a/src/openhuman/memory/store/memory_trait.rs +++ b/src/openhuman/memory/store/memory_trait.rs @@ -256,6 +256,25 @@ impl Memory for UnifiedMemory { Ok(out) } + async fn recall_relevant_by_vector( + &self, + namespace: &str, + query: &str, + limit: usize, + min_vector_similarity: f64, + ) -> anyhow::Result> { + let hits = self + .query_namespace_hits(namespace, query, limit as u32) + .await + .map_err(anyhow::Error::msg)?; + Ok(hits + .into_iter() + .filter(|h| h.score_breakdown.vector_similarity >= min_vector_similarity) + .map(|h| h.content) + .filter(|c| !c.trim().is_empty()) + .collect()) + } + async fn get(&self, namespace: &str, key: &str) -> anyhow::Result> { let ns = if namespace.trim().is_empty() { GLOBAL_NAMESPACE.to_string() diff --git a/src/openhuman/memory/store/unified/query_tests.rs b/src/openhuman/memory/store/unified/query_tests.rs index 65a02bf897..328601e2c3 100644 --- a/src/openhuman/memory/store/unified/query_tests.rs +++ b/src/openhuman/memory/store/unified/query_tests.rs @@ -6,7 +6,7 @@ use serde_json::json; use tempfile::TempDir; use crate::openhuman::embeddings::NoopEmbedding; -use crate::openhuman::memory::{NamespaceDocumentInput, UnifiedMemory}; +use crate::openhuman::memory::{Memory, NamespaceDocumentInput, UnifiedMemory}; #[tokio::test] async fn graph_duplicate_upsert_aggregates_evidence_count() { @@ -587,3 +587,90 @@ async fn vector_recall_skips_dimension_mismatch_for_untagged_rows() { "dimension-mismatched legacy vectors must be skipped, not scored 0" ); } + +// ── recall_relevant_by_vector — Lane B situational-pref relevance gate ───────── + +/// Embedder whose vector depends on keywords in the text, so a query can be +/// genuinely relevant (high cosine) or irrelevant (zero) to a stored pref. +struct KeywordEmbedder; + +#[async_trait] +impl EmbeddingProvider for KeywordEmbedder { + fn name(&self) -> &str { + "keyword-stub" + } + fn model_id(&self) -> &str { + "keyword-stub" + } + fn dimensions(&self) -> usize { + 2 + } + async fn embed(&self, texts: &[&str]) -> anyhow::Result>> { + Ok(texts + .iter() + .map(|t| { + let lower = t.to_lowercase(); + vec![ + if lower.contains("rust") { 1.0 } else { 0.0 }, + if lower.contains("email") { 1.0 } else { 0.0 }, + ] + }) + .collect()) + } +} + +fn situational_doc(key: &str, content: &str) -> NamespaceDocumentInput { + NamespaceDocumentInput { + namespace: "user_pref_situational".to_string(), + key: key.to_string(), + title: key.to_string(), + content: content.to_string(), + source_type: "pref".to_string(), + priority: "medium".to_string(), + tags: vec![], + metadata: json!({}), + category: "core".to_string(), + session_id: None, + document_id: None, + } +} + +#[tokio::test] +async fn recall_relevant_by_vector_gates_on_similarity() { + let tmp = TempDir::new().unwrap(); + let memory = UnifiedMemory::new(tmp.path(), Arc::new(KeywordEmbedder), None).unwrap(); + + // Two situational prefs that embed onto orthogonal axes. + memory + .upsert_document(situational_doc( + "rust_style", + "When writing rust, prefer explicit error handling.", + )) + .await + .unwrap(); + memory + .upsert_document(situational_doc( + "email_tone", + "Be formal in email to my manager.", + )) + .await + .unwrap(); + + // A rust-related message recalls only the rust pref. + let hits = memory + .recall_relevant_by_vector("user_pref_situational", "help me with my rust code", 5, 0.5) + .await + .unwrap(); + assert_eq!(hits.len(), 1, "only the relevant pref should pass the gate"); + assert!(hits[0].contains("explicit error handling")); + + // An unrelated message clears the gate to nothing — no block injected. + let none = memory + .recall_relevant_by_vector("user_pref_situational", "what is the weather today", 5, 0.5) + .await + .unwrap(); + assert!( + none.is_empty(), + "an unrelated message must surface no situational preferences" + ); +} diff --git a/src/openhuman/memory/traits.rs b/src/openhuman/memory/traits.rs index e28a44df38..0926c8910f 100644 --- a/src/openhuman/memory/traits.rs +++ b/src/openhuman/memory/traits.rs @@ -130,6 +130,27 @@ pub trait Memory: Send + Sync { opts: RecallOpts<'_>, ) -> anyhow::Result>; + /// Recall documents in `namespace` semantically relevant to `query`, keeping + /// only those whose *vector* similarity to the query is at least + /// `min_vector_similarity`. Returns rendered content bodies, most-relevant + /// first. + /// + /// Unlike [`Self::recall`] (which ranks on a combined keyword + vector + + /// freshness score), this gates on the vector component alone, so an + /// unrelated query surfaces nothing — the behaviour Lane-B situational + /// preferences need. Default returns empty so keyword-only and mock backends + /// opt out; the unified store overrides it. + async fn recall_relevant_by_vector( + &self, + namespace: &str, + query: &str, + limit: usize, + min_vector_similarity: f64, + ) -> anyhow::Result> { + let _ = (namespace, query, limit, min_vector_similarity); + Ok(Vec::new()) + } + /// Retrieves a specific memory entry by exact (namespace, key). async fn get(&self, namespace: &str, key: &str) -> anyhow::Result>; From 8d4ffef8c9d4e0f23fece392a59e0d64ce86e360 Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Fri, 22 May 2026 08:34:35 +0200 Subject: [PATCH 04/10] feat(memory): chat-affirmed contradiction surfacing on save_preference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same-topic contradictions were already handled (ON CONFLICT REPLACE + recategorise clears the other lane). For cross-topic semantic contradictions, rather than a separate hidden LLM judge, surface related existing preferences to the chat agent that captured the preference — it is already in the loop and can resolve it. - recall_relevant_by_vector now returns (topic, value) pairs so callers can act on the matched entry by topic. - on save, recall_related_preferences finds prefs across both lanes within a high similarity threshold (excluding the just-saved topic) and lists them in the tool result, telling the model to overwrite/remove a conflicting one or leave them. - tests: related pref surfaced for a near-duplicate; nothing for an unrelated pref. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/openhuman/memory/preferences.rs | 38 ++++++++ src/openhuman/memory/store/memory_trait.rs | 6 +- .../memory/store/unified/query_tests.rs | 3 +- src/openhuman/memory/traits.rs | 7 +- .../tools/impl/agent/save_preference.rs | 33 ++++++- .../tools/impl/agent/save_preference_tests.rs | 95 +++++++++++++++++++ 6 files changed, 171 insertions(+), 11 deletions(-) diff --git a/src/openhuman/memory/preferences.rs b/src/openhuman/memory/preferences.rs index 140f0c56dd..acf624b60f 100644 --- a/src/openhuman/memory/preferences.rs +++ b/src/openhuman/memory/preferences.rs @@ -78,4 +78,42 @@ pub async fn recall_situational_preferences(memory: &Arc, query: &st ) .await .unwrap_or_default() + .into_iter() + .map(|(_topic, value)| value) + .collect() +} + +/// Minimum similarity for an existing preference to be flagged as a possible +/// contradiction of a newly-saved one. Higher than the Lane-B recall floor — we +/// only surface genuinely-close matches as contradiction candidates. Tunable. +pub const CONTRADICTION_SIMILARITY: f64 = 0.6; + +/// Find existing preferences (across both lanes) semantically close to `value`, +/// excluding `exclude_topic` (the just-saved one). Returns `(topic, value)` +/// pairs so the chat agent — which captured the preference in the first place — +/// can resolve a contradiction itself: overwrite the conflicting topic or remove +/// it. No separate model call; the conversation affirms it. +pub async fn recall_related_preferences( + memory: &Arc, + value: &str, + exclude_topic: &str, + limit: usize, +) -> Vec<(String, String)> { + if value.trim().is_empty() { + return Vec::new(); + } + let mut out = Vec::new(); + for ns in [USER_PREF_GENERAL_NAMESPACE, USER_PREF_SITUATIONAL_NAMESPACE] { + if let Ok(hits) = memory + .recall_relevant_by_vector(ns, value, limit, CONTRADICTION_SIMILARITY) + .await + { + for (topic, val) in hits { + if topic != exclude_topic { + out.push((topic, val)); + } + } + } + } + out } diff --git a/src/openhuman/memory/store/memory_trait.rs b/src/openhuman/memory/store/memory_trait.rs index 28cb4a9cbb..d0ca3336e6 100644 --- a/src/openhuman/memory/store/memory_trait.rs +++ b/src/openhuman/memory/store/memory_trait.rs @@ -262,7 +262,7 @@ impl Memory for UnifiedMemory { query: &str, limit: usize, min_vector_similarity: f64, - ) -> anyhow::Result> { + ) -> anyhow::Result> { let hits = self .query_namespace_hits(namespace, query, limit as u32) .await @@ -270,8 +270,8 @@ impl Memory for UnifiedMemory { Ok(hits .into_iter() .filter(|h| h.score_breakdown.vector_similarity >= min_vector_similarity) - .map(|h| h.content) - .filter(|c| !c.trim().is_empty()) + .filter(|h| !h.content.trim().is_empty()) + .map(|h| (h.key, h.content)) .collect()) } diff --git a/src/openhuman/memory/store/unified/query_tests.rs b/src/openhuman/memory/store/unified/query_tests.rs index 328601e2c3..21ac37a1ff 100644 --- a/src/openhuman/memory/store/unified/query_tests.rs +++ b/src/openhuman/memory/store/unified/query_tests.rs @@ -662,7 +662,8 @@ async fn recall_relevant_by_vector_gates_on_similarity() { .await .unwrap(); assert_eq!(hits.len(), 1, "only the relevant pref should pass the gate"); - assert!(hits[0].contains("explicit error handling")); + assert_eq!(hits[0].0, "rust_style"); + assert!(hits[0].1.contains("explicit error handling")); // An unrelated message clears the gate to nothing — no block injected. let none = memory diff --git a/src/openhuman/memory/traits.rs b/src/openhuman/memory/traits.rs index 0926c8910f..e2d785d0da 100644 --- a/src/openhuman/memory/traits.rs +++ b/src/openhuman/memory/traits.rs @@ -132,8 +132,9 @@ pub trait Memory: Send + Sync { /// Recall documents in `namespace` semantically relevant to `query`, keeping /// only those whose *vector* similarity to the query is at least - /// `min_vector_similarity`. Returns rendered content bodies, most-relevant - /// first. + /// `min_vector_similarity`. Returns `(key, content)` pairs, most-relevant + /// first — the key lets callers act on the matched entry (e.g. overwrite a + /// contradicting preference by its topic). /// /// Unlike [`Self::recall`] (which ranks on a combined keyword + vector + /// freshness score), this gates on the vector component alone, so an @@ -146,7 +147,7 @@ pub trait Memory: Send + Sync { query: &str, limit: usize, min_vector_similarity: f64, - ) -> anyhow::Result> { + ) -> anyhow::Result> { let _ = (namespace, query, limit, min_vector_similarity); Ok(Vec::new()) } diff --git a/src/openhuman/tools/impl/agent/save_preference.rs b/src/openhuman/tools/impl/agent/save_preference.rs index 46fcd2ea98..97d9498934 100644 --- a/src/openhuman/tools/impl/agent/save_preference.rs +++ b/src/openhuman/tools/impl/agent/save_preference.rs @@ -244,10 +244,35 @@ impl Tool for SavePreferenceTool { topic, category.as_str() ); - Ok(ToolResult::success(format!( - "Saved {} preference: {topic} = {value}", - category.as_str() - ))) + // Surface semantically-related existing preferences so the chat + // agent (which captured this preference) can spot and resolve a + // contradiction itself — no separate model call. + let related = crate::openhuman::memory::preferences::recall_related_preferences( + &self.memory, + value, + topic, + 4, + ) + .await; + let mut msg = format!("Saved {} preference: {topic} = {value}", category.as_str()); + if !related.is_empty() { + tracing::info!( + "[tool][save_preference] {} related preference(s) surfaced for contradiction check", + related.len() + ); + msg.push_str( + "\n\nExisting preferences related to this one — check for contradictions:", + ); + for (other_topic, other_value) in &related { + msg.push_str(&format!("\n- {other_topic}: {other_value}")); + } + msg.push_str( + "\n\nIf any of these conflicts with what was just saved, resolve it now: \ + overwrite that topic with save_preference, or remove it with memory_forget. \ + Otherwise leave them as-is.", + ); + } + Ok(ToolResult::success(msg)) } Err(e) => { tracing::error!( diff --git a/src/openhuman/tools/impl/agent/save_preference_tests.rs b/src/openhuman/tools/impl/agent/save_preference_tests.rs index a2f9c254a4..4e0da7b9f8 100644 --- a/src/openhuman/tools/impl/agent/save_preference_tests.rs +++ b/src/openhuman/tools/impl/agent/save_preference_tests.rs @@ -176,3 +176,98 @@ async fn recategorising_moves_pref_between_namespaces() { "the general-scope copy must be cleared when re-categorised" ); } + +// ── Contradiction surfacing (chat-affirmed) ────────────────────────────────── + +use async_trait::async_trait; + +/// Keyword-sensitive embedder so prefs about the same theme embed close together +/// (high cosine) and unrelated ones don't. +struct KwEmbedder; + +#[async_trait] +impl crate::openhuman::embeddings::EmbeddingProvider for KwEmbedder { + fn name(&self) -> &str { + "kw" + } + fn model_id(&self) -> &str { + "kw" + } + fn dimensions(&self) -> usize { + 2 + } + async fn embed(&self, texts: &[&str]) -> anyhow::Result>> { + Ok(texts + .iter() + .map(|t| { + let l = t.to_lowercase(); + vec![ + if l.contains("terse") || l.contains("verbose") || l.contains("detail") { + 1.0 + } else { + 0.0 + }, + if l.contains("rust") { 1.0 } else { 0.0 }, + ] + }) + .collect()) + } +} + +fn kw_mem() -> (TempDir, Arc) { + let tmp = TempDir::new().unwrap(); + let mem = UnifiedMemory::new(tmp.path(), Arc::new(KwEmbedder), None).unwrap(); + (tmp, Arc::new(mem)) +} + +#[tokio::test] +async fn save_surfaces_related_preference_for_contradiction_check() { + let (_tmp, mem) = kw_mem(); + let tool = SavePreferenceTool::new(mem.clone(), test_security()); + + tool.execute(json!({"topic": "verbosity", "value": "always be terse", "category": "general"})) + .await + .unwrap(); + + // A semantically-related pref under a different topic. + let r = tool + .execute(json!({ + "topic": "explanation_style", + "value": "give detailed verbose explanations", + "category": "general" + })) + .await + .unwrap(); + assert!(!r.is_error); + assert!( + r.output().contains("verbosity") && r.output().contains("always be terse"), + "expected the related pref to be surfaced for a contradiction check, got: {}", + r.output() + ); +} + +#[tokio::test] +async fn save_unrelated_preference_surfaces_nothing() { + let (_tmp, mem) = kw_mem(); + let tool = SavePreferenceTool::new(mem.clone(), test_security()); + + tool.execute(json!({"topic": "verbosity", "value": "always be terse", "category": "general"})) + .await + .unwrap(); + + // An unrelated pref (rust) — no contradiction note. + let r = tool + .execute(json!({ + "topic": "rust_edition", + "value": "use rust 2021 edition", + "category": "situational" + })) + .await + .unwrap(); + assert!(!r.is_error); + assert!( + !r.output().contains("check for contradictions"), + "an unrelated pref should surface no related prefs, got: {}", + r.output() + ); +} From 06fd3f29967c0ac832f61ed0d5c7151b77c1a7c6 Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Fri, 22 May 2026 08:39:46 +0200 Subject: [PATCH 05/10] feat(memory): demote inferred user_profile facets from prompt injection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The system prompt's standing-preferences block now sources exclusively from the explicit two-lane store (user_pref_general) in BOTH the explicit-only and full learning paths. The inferred user_profile facets (Archivist heuristic + tree stability detector) are no longer injected as ground truth — a high-confidence facet should be proposed to the user and pinned via save_preference, not silently treated as a standing preference. - fetch_learned_context learning path: the user_profile field loads general prefs instead of the inferred user_profile namespace. - tests: load_general_preferences returns values (not topic keys), newest-first, capped; the explicit-path fetch_learned_context test now uses the two-lane store. Follow-up: a proactive suggestion surface that promotes high-confidence inferred facets into user_pref on user confirmation. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/openhuman/agent/harness/session/turn.rs | 25 +++++------ .../agent/harness/session/turn_tests.rs | 33 +++++++------- src/openhuman/memory/preferences.rs | 43 +++++++++++++++++++ 3 files changed, 70 insertions(+), 31 deletions(-) diff --git a/src/openhuman/agent/harness/session/turn.rs b/src/openhuman/agent/harness/session/turn.rs index b204d9ee3d..ee3c420de9 100644 --- a/src/openhuman/agent/harness/session/turn.rs +++ b/src/openhuman/agent/harness/session/turn.rs @@ -1571,15 +1571,16 @@ impl Agent { .await .unwrap_or_default(); - let profile_entries = self - .memory - .list( - Some("user_profile"), - Some(&MemoryCategory::Custom("user_profile".into())), - None, - ) - .await - .unwrap_or_default(); + // Standing preferences come from the explicit two-lane store (Lane A), + // not the inferred `user_profile` facets — those are demoted: no longer + // injected as ground truth. A high-confidence inferred facet should be + // *proposed* to the user (and pinned via `save_preference` on + // confirmation), not silently treated as a standing preference. + let general = crate::openhuman::memory::preferences::load_general_preferences( + &self.memory, + crate::openhuman::memory::preferences::STANDING_PREFS_LIMIT, + ) + .await; // Explicit user reflections — privileged memory class. Pulled // separately from observations/patterns so the prompt assembly @@ -1625,11 +1626,7 @@ impl Agent { .take(3) .map(|e| sanitize_learned_entry(&e.content)) .collect(), - user_profile: profile_entries - .iter() - .take(20) - .map(|e| sanitize_learned_entry(&e.content)) - .collect(), + user_profile: general, // Cap reflections at 10 to keep the privileged section // bounded — the issue requires reflections improve context // rather than flood it. Newest first. diff --git a/src/openhuman/agent/harness/session/turn_tests.rs b/src/openhuman/agent/harness/session/turn_tests.rs index ee55a518a2..311e2bfe24 100644 --- a/src/openhuman/agent/harness/session/turn_tests.rs +++ b/src/openhuman/agent/harness/session/turn_tests.rs @@ -792,7 +792,7 @@ async fn execute_tool_call_applies_inline_result_budget() { // flag combinations: // 1. both flags off → empty context // 2. explicit_preferences_enabled=true, learning_enabled=false -// → only pinned user_profile entries returned, no inference data +// → only general user_pref entries returned, no inference data // 3. learning_enabled=true → full path (existing tests cover this; we only // verify that explicit entries are included as well) // @@ -860,24 +860,26 @@ async fn fetch_learned_context_returns_empty_when_both_flags_off() { } #[tokio::test] -async fn fetch_learned_context_returns_pinned_prefs_when_explicit_flag_on_learning_off() { +async fn fetch_learned_context_returns_general_prefs_when_explicit_flag_on_learning_off() { let tmp = tempfile::TempDir::new().unwrap(); let mem = make_real_memory(tmp.path()); - // Store two pinned preferences via the same key format RememberPreferenceTool uses. + // Store two general preferences in the two-lane store (where save_preference + // writes them). The explicit path now reads `user_pref_general`, not the + // legacy `user_profile` pinned namespace. mem.store( - "user_profile", - "pinned/tooling/package_manager", - "[pinned] (class=tooling) package_manager: pnpm", + crate::openhuman::memory::preferences::USER_PREF_GENERAL_NAMESPACE, + "package_manager", + "Use pnpm for package management.", crate::openhuman::memory::MemoryCategory::Core, None, ) .await .unwrap(); mem.store( - "user_profile", - "pinned/style/verbosity", - "[pinned] (class=style) verbosity: terse", + crate::openhuman::memory::preferences::USER_PREF_GENERAL_NAMESPACE, + "verbosity", + "Keep replies terse.", crate::openhuman::memory::MemoryCategory::Core, None, ) @@ -896,20 +898,17 @@ async fn fetch_learned_context_returns_pinned_prefs_when_explicit_flag_on_learni assert_eq!( learned.user_profile.len(), 2, - "explicit flag on, learning off: expected 2 pinned preferences, got: {:?}", + "explicit flag on, learning off: expected 2 general preferences, got: {:?}", learned.user_profile ); assert!( - learned - .user_profile - .iter() - .any(|s| s.contains("package_manager")), - "package_manager preference must appear in user_profile: {:?}", + learned.user_profile.iter().any(|s| s.contains("pnpm")), + "package_manager preference value must appear in user_profile: {:?}", learned.user_profile ); assert!( - learned.user_profile.iter().any(|s| s.contains("verbosity")), - "verbosity preference must appear in user_profile: {:?}", + learned.user_profile.iter().any(|s| s.contains("terse")), + "verbosity preference value must appear in user_profile: {:?}", learned.user_profile ); // Inference-derived data must remain empty — the stack was NOT engaged. diff --git a/src/openhuman/memory/preferences.rs b/src/openhuman/memory/preferences.rs index acf624b60f..6db2a888db 100644 --- a/src/openhuman/memory/preferences.rs +++ b/src/openhuman/memory/preferences.rs @@ -117,3 +117,46 @@ pub async fn recall_related_preferences( } out } + +#[cfg(test)] +mod tests { + use super::*; + use crate::openhuman::embeddings::NoopEmbedding; + use crate::openhuman::memory::{MemoryCategory, UnifiedMemory}; + use tempfile::TempDir; + + #[tokio::test] + async fn load_general_preferences_returns_values_newest_first_capped() { + let tmp = TempDir::new().unwrap(); + let mem: Arc = + Arc::new(UnifiedMemory::new(tmp.path(), Arc::new(NoopEmbedding), None).unwrap()); + + mem.store( + USER_PREF_GENERAL_NAMESPACE, + "reply_language", + "Reply in British English.", + MemoryCategory::Core, + None, + ) + .await + .unwrap(); + mem.store( + USER_PREF_GENERAL_NAMESPACE, + "tone", + "Be terse.", + MemoryCategory::Core, + None, + ) + .await + .unwrap(); + + let general = load_general_preferences(&mem, 10).await; + // Returns the values (bodies), not the topic keys. + assert!(general.iter().any(|v| v.contains("British English"))); + assert!(general.iter().any(|v| v.contains("Be terse"))); + assert!(!general.iter().any(|v| v == "reply_language")); + + // The limit caps the block. + assert_eq!(load_general_preferences(&mem, 1).await.len(), 1); + } +} From b3f628b7ff44e6daa2abb94e60553f85e52777d2 Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Fri, 22 May 2026 17:04:38 +0200 Subject: [PATCH 06/10] fix(tools): route preference-intent to save_preference, not memory_store The chat model preferentially called memory_store (which advertised storing 'preferences') for user preferences, so they landed in an arbitrary namespace that no preference lane reads. Narrow memory_store's description to facts/notes only and explicitly defer all preferences to save_preference (surfaced during live testing). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/openhuman/tools/impl/memory/store.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/openhuman/tools/impl/memory/store.rs b/src/openhuman/tools/impl/memory/store.rs index 22b5901b84..a00808e383 100644 --- a/src/openhuman/tools/impl/memory/store.rs +++ b/src/openhuman/tools/impl/memory/store.rs @@ -26,7 +26,10 @@ impl Tool for MemoryStoreTool { } fn description(&self) -> &str { - "Store a fact, preference, or note in a namespace. Requires explicit namespace (e.g. global, background, autocomplete, skill-telegram)." + "Store a general fact or note in a namespace (e.g. global, background, autocomplete, skill-{id}). \ + Do NOT use this for user preferences — for any preference (how the user wants you to behave, \ + their tastes, settings, standing instructions) call `save_preference` instead, which routes it \ + to the preference store the assistant actually reads. Requires an explicit namespace." } fn parameters_schema(&self) -> serde_json::Value { From 39e9dc601f1860f39b7de781203c12176e84c2d5 Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Fri, 22 May 2026 17:09:08 +0200 Subject: [PATCH 07/10] feat(agent): expose save_preference in the orchestrator toolset save_preference was registered globally (ops.rs) but absent from the orchestrator's named tool allowlist, so the model was never offered it and fell back to memory_store even when explicitly asked to use it. Add it next to memory_store. Surfaced during live testing of the two-lane preferences. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/openhuman/agent/agents/orchestrator/agent.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/src/openhuman/agent/agents/orchestrator/agent.toml b/src/openhuman/agent/agents/orchestrator/agent.toml index 78b1dd0189..001417c02e 100644 --- a/src/openhuman/agent/agents/orchestrator/agent.toml +++ b/src/openhuman/agent/agents/orchestrator/agent.toml @@ -101,6 +101,7 @@ hint = "chat" named = [ "query_memory", "memory_store", + "save_preference", "memory_forget", "memory_tree", # WhatsApp local-data tools (issue #1341). The scanner ingests chats From a399dd577ba9923418ec556a6cf40b603711f5da Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Fri, 22 May 2026 20:47:47 +0200 Subject: [PATCH 08/10] fix(embeddings): cloud embedder honours OPENHUMAN_WORKSPACE for session lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OpenHumanCloudEmbedding::state_dir() fell back to ~/.openhuman when openhuman_dir was None (how the memory store builds its embedder), ignoring OPENHUMAN_WORKSPACE. On any non-default workspace the session JWT was never found, so resolve_bearer() bailed, embed() errored, and vector_chunks rows were written with NULL embedding/model_signature/dim — silently disabling all semantic recall, including Lane-B situational preferences. Honour OPENHUMAN_WORKSPACE before the home fallback, matching how the chat provider resolves its dir. Surfaced while live-testing the two-lane preferences. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/openhuman/embeddings/cloud.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/openhuman/embeddings/cloud.rs b/src/openhuman/embeddings/cloud.rs index ee77d6fe4b..dece6e6f7e 100644 --- a/src/openhuman/embeddings/cloud.rs +++ b/src/openhuman/embeddings/cloud.rs @@ -60,6 +60,18 @@ impl OpenHumanCloudEmbedding { fn state_dir(&self) -> PathBuf { self.openhuman_dir.clone().unwrap_or_else(|| { + // Honor OPENHUMAN_WORKSPACE (where auth-profiles.json lives) before + // falling back to ~/.openhuman, so the cloud embedder resolves the + // session JWT from the same directory the chat provider does. Without + // this, any non-default workspace (OPENHUMAN_WORKSPACE set, e.g. tests + // / multi-instance) silently has no session for embeddings — + // resolve_bearer() bails, embed() errors, and vectors are dropped. + if let Some(ws) = std::env::var_os("OPENHUMAN_WORKSPACE") + .filter(|s| !s.is_empty()) + .map(PathBuf::from) + { + return ws; + } directories::UserDirs::new() .map(|d| d.home_dir().join(".openhuman")) .unwrap_or_else(|| PathBuf::from(".openhuman")) From 2ee733c38d287bb886165437a90087986ba4a3c8 Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Fri, 22 May 2026 21:53:36 +0200 Subject: [PATCH 09/10] docs(memory): coverage matrix + about_app capability for two-lane preferences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - TEST-COVERAGE-MATRIX.md §8.4: six rows mapping the two-lane preference features to their Rust unit tests. - about_app catalog: add the intelligence.remember_preferences capability. - Add fetch_learned_context_loads_general_prefs_when_learning_enabled test covering the demotion path in the learning-enabled branch. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/TEST-COVERAGE-MATRIX.md | 11 ++++++++ src/openhuman/about_app/catalog.rs | 13 ++++++++++ .../agent/harness/session/turn_tests.rs | 26 +++++++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/docs/TEST-COVERAGE-MATRIX.md b/docs/TEST-COVERAGE-MATRIX.md index 9704f6c85e..a58edf5d75 100644 --- a/docs/TEST-COVERAGE-MATRIX.md +++ b/docs/TEST-COVERAGE-MATRIX.md @@ -294,6 +294,17 @@ Canonical mapping of every product feature to its test source(s). Drives gap-fil | 8.3.8 | Drill-Down Isolates Children | RU | `src/openhuman/memory/tree/retrieval/benchmarks.rs::bench_drill_down_isolates_children` | ✅ | Verifies query_topic does not cross scope boundaries | | 8.3.9 | Scale Ingest 20 Sources No Real Data | RU | `src/openhuman/memory/tree/retrieval/benchmarks.rs::bench_scale_ingest_20_sources_no_real_data` | ✅ | Verifies retrieval correctness at scale with synthetic data | +### 8.4 Explicit User Preferences (Two-Lane) + +| ID | Feature | Layer | Test path(s) | Status | Notes | +| ----- | ------------------------------------------ | ----- | ----------------------------------------------------------------------------------------------------------------- | ------ | ---------------------------------------------------------------------- | +| 8.4.1 | Save Preference (general / situational) | RU | `src/openhuman/tools/impl/agent/save_preference_tests.rs` | ✅ | `save_preference` tool → `user_pref_{general,situational}`, topic-keyed | +| 8.4.2 | Lane A — Standing Prefs in System Prompt | RU | `src/openhuman/learning/prompt_sections.rs`, `src/openhuman/agent/harness/session/turn_tests.rs` | ✅ | General prefs rendered into the system prompt at thread start | +| 8.4.3 | Lane B — Situational Recall (vector-gated) | RU | `src/openhuman/memory/store/unified/query_tests.rs::recall_relevant_by_vector_gates_on_similarity` | ✅ | Per-turn; relevant query injects, unrelated suppresses | +| 8.4.4 | Same-Topic Contradiction (replace) | RU | `src/openhuman/tools/impl/agent/save_preference_tests.rs::recategorising_moves_pref_between_namespaces` | ✅ | `ON CONFLICT REPLACE`; a topic lives in exactly one scope | +| 8.4.5 | Cross-Topic Contradiction Surfacing | RU | `src/openhuman/tools/impl/agent/save_preference_tests.rs::save_surfaces_related_preference_for_contradiction_check` | ✅ | Related prefs surfaced in the tool result for the chat agent to resolve | +| 8.4.6 | vector_chunks Model-Signature Recall Guard | RU | `src/openhuman/memory/store/unified/query_tests.rs::vector_recall_excludes_other_model_signature` | ✅ | Excludes cross-model vectors; dim-guards legacy rows | + --- ## 9. Automation Engine diff --git a/src/openhuman/about_app/catalog.rs b/src/openhuman/about_app/catalog.rs index b15b1bf08a..2e26b9a77e 100644 --- a/src/openhuman/about_app/catalog.rs +++ b/src/openhuman/about_app/catalog.rs @@ -1270,6 +1270,19 @@ const CAPABILITIES: &[Capability] = &[ status: CapabilityStatus::Beta, privacy: None, }, + Capability { + id: "intelligence.remember_preferences", + name: "Remember Preferences", + domain: "memory", + category: CapabilityCategory::Intelligence, + description: "Remember preferences you state in chat and apply them automatically — \ + general preferences shape every reply (tone, language, standing habits); \ + situational ones surface only when relevant to your current message.", + how_to: "State a preference in chat, e.g. \"always reply in British English\" or \ + \"when writing Rust, prefer Result over unwrap\".", + status: CapabilityStatus::Stable, + privacy: LOCAL_RAW, + }, ]; static VALIDATED: OnceLock<()> = OnceLock::new(); diff --git a/src/openhuman/agent/harness/session/turn_tests.rs b/src/openhuman/agent/harness/session/turn_tests.rs index 311e2bfe24..9ce2d92ff1 100644 --- a/src/openhuman/agent/harness/session/turn_tests.rs +++ b/src/openhuman/agent/harness/session/turn_tests.rs @@ -956,3 +956,29 @@ async fn fetch_learned_context_explicit_flag_off_learning_off_returns_empty_even learned.user_profile ); } + +#[tokio::test] +async fn fetch_learned_context_loads_general_prefs_when_learning_enabled() { + let tmp = tempfile::TempDir::new().unwrap(); + let mem = make_real_memory(tmp.path()); + mem.store( + crate::openhuman::memory::preferences::USER_PREF_GENERAL_NAMESPACE, + "tone", + "Be concise and direct.", + crate::openhuman::memory::MemoryCategory::Core, + None, + ) + .await + .unwrap(); + + // learning_enabled=true → full path, which now also sources standing prefs + // from the explicit user_pref_general store (inferred facets are demoted, so + // they are no longer injected as ground truth). + let agent = make_agent_with_memory(mem, tmp.path().to_path_buf(), true, true); + let learned = agent.fetch_learned_context().await; + assert!( + learned.user_profile.iter().any(|s| s.contains("concise")), + "learning path must inject explicit general prefs into user_profile: {:?}", + learned.user_profile + ); +} From b816f68b305b52745acbd76a8cbc3fc75553abe0 Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Fri, 22 May 2026 22:12:06 +0200 Subject: [PATCH 10/10] fix(preferences): address CodeRabbit review on save_preference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Reject secret-like values via memory::safety::has_likely_secret before any write (same guard memory_store uses) — a credential pasted as a 'preference' is no longer stored verbatim. - Clear the opposite-lane copy only *after* the new store succeeds, so a failed store can't leave the user with neither copy (data-loss fix). - recall_related_preferences: spend a shared 'remaining' budget across both namespaces so the total surfaced never exceeds the caller-requested limit. - Add secret_like_value_is_rejected_before_write test. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/openhuman/memory/preferences.rs | 13 ++++++- .../tools/impl/agent/save_preference.rs | 39 +++++++++++++------ .../tools/impl/agent/save_preference_tests.rs | 21 ++++++++++ 3 files changed, 61 insertions(+), 12 deletions(-) diff --git a/src/openhuman/memory/preferences.rs b/src/openhuman/memory/preferences.rs index 6db2a888db..b02da13c21 100644 --- a/src/openhuman/memory/preferences.rs +++ b/src/openhuman/memory/preferences.rs @@ -103,14 +103,25 @@ pub async fn recall_related_preferences( return Vec::new(); } let mut out = Vec::new(); + // `limit` is a global cap across *both* lanes, not per-namespace — spend a + // shared budget so the total surfaced for one contradiction check can never + // exceed what the caller asked for. + let mut remaining = limit; for ns in [USER_PREF_GENERAL_NAMESPACE, USER_PREF_SITUATIONAL_NAMESPACE] { + if remaining == 0 { + break; + } if let Ok(hits) = memory - .recall_relevant_by_vector(ns, value, limit, CONTRADICTION_SIMILARITY) + .recall_relevant_by_vector(ns, value, remaining, CONTRADICTION_SIMILARITY) .await { for (topic, val) in hits { if topic != exclude_topic { out.push((topic, val)); + remaining = remaining.saturating_sub(1); + if remaining == 0 { + break; + } } } } diff --git a/src/openhuman/tools/impl/agent/save_preference.rs b/src/openhuman/tools/impl/agent/save_preference.rs index 97d9498934..922ed41c1f 100644 --- a/src/openhuman/tools/impl/agent/save_preference.rs +++ b/src/openhuman/tools/impl/agent/save_preference.rs @@ -23,7 +23,7 @@ use std::sync::Arc; use async_trait::async_trait; use serde_json::json; -use crate::openhuman::memory::{Memory, MemoryCategory}; +use crate::openhuman::memory::{safety, Memory, MemoryCategory}; use crate::openhuman::security::policy::ToolOperation; use crate::openhuman::security::SecurityPolicy; use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolResult}; @@ -211,19 +211,24 @@ impl Tool for SavePreferenceTool { if value.is_empty() { return Ok(ToolResult::error("value cannot be empty".to_string())); } - - let namespace = category.namespace(); - - // A topic lives in exactly one scope — clear any prior copy in the other - // namespace so a re-categorised preference doesn't linger in both lanes. - if let Err(e) = self.memory.forget(category.other_namespace(), topic).await { - tracing::debug!( - "[tool][save_preference] clearing other-scope copy failed (non-fatal) ns={} topic={}: {e}", - category.other_namespace(), - topic + // Same secret guard `memory_store` applies — a credential pasted as a + // "preference" would otherwise be stored verbatim and later surfaced or + // injected. Reject before any write. + if safety::has_likely_secret(value) { + tracing::warn!( + "[tool][save_preference] rejected secret-like value topic={} value_chars={}", + topic, + value.len() ); + return Ok(ToolResult::error( + "Refusing to store content that looks like a secret. Remove credentials or \ + tokens and try again." + .to_string(), + )); } + let namespace = category.namespace(); + tracing::debug!( "[tool][save_preference] storing namespace={} topic={} category={} value_len={}", namespace, @@ -244,6 +249,18 @@ impl Tool for SavePreferenceTool { topic, category.as_str() ); + // A topic lives in exactly one scope. Now that the new write has + // succeeded, clear any prior copy in the other namespace so a + // re-categorised preference doesn't linger in both lanes. Done + // *after* the store (not before) so a store failure can never + // leave the user with neither copy. + if let Err(e) = self.memory.forget(category.other_namespace(), topic).await { + tracing::debug!( + "[tool][save_preference] clearing other-scope copy failed (non-fatal) ns={} topic={}: {e}", + category.other_namespace(), + topic + ); + } // Surface semantically-related existing preferences so the chat // agent (which captured this preference) can spot and resolve a // contradiction itself — no separate model call. diff --git a/src/openhuman/tools/impl/agent/save_preference_tests.rs b/src/openhuman/tools/impl/agent/save_preference_tests.rs index 4e0da7b9f8..98b3803126 100644 --- a/src/openhuman/tools/impl/agent/save_preference_tests.rs +++ b/src/openhuman/tools/impl/agent/save_preference_tests.rs @@ -123,6 +123,27 @@ async fn empty_value_returns_error() { assert!(r.is_error); } +#[tokio::test] +async fn secret_like_value_is_rejected_before_write() { + let (_tmp, mem) = test_mem(); + let tool = SavePreferenceTool::new(mem.clone(), test_security()); + let r = tool + .execute(json!({ + "topic": "api", + "value": "api_key=sk-123456789012345678901234567890", + "category": "general", + })) + .await + .unwrap(); + assert!(r.is_error); + assert!(r.output().contains("looks like a secret")); + // Nothing persisted in either lane. + assert!(keys_in(&mem, USER_PREF_GENERAL_NAMESPACE).await.is_empty()); + assert!(keys_in(&mem, USER_PREF_SITUATIONAL_NAMESPACE) + .await + .is_empty()); +} + // ── Storage behaviour ───────────────────────────────────────────────────────── #[tokio::test]