From d812bf9da78055e91a8332c8875dc13629c822ed Mon Sep 17 00:00:00 2001 From: Steven Enamakel <31011319+senamakel@users.noreply.github.com> Date: Mon, 4 May 2026 00:09:50 -0700 Subject: [PATCH] fix(agent): drop redundant [Memory context] recall injection (#1173) --- src/openhuman/agent/memory_loader.rs | 49 ++++++---------------------- tests/agent_memory_loader_public.rs | 31 ++++++++++++------ 2 files changed, 31 insertions(+), 49 deletions(-) diff --git a/src/openhuman/agent/memory_loader.rs b/src/openhuman/agent/memory_loader.rs index a6c99effd..cfa907e8e 100644 --- a/src/openhuman/agent/memory_loader.rs +++ b/src/openhuman/agent/memory_loader.rs @@ -1,7 +1,6 @@ use crate::openhuman::memory::Memory; use async_trait::async_trait; use serde::{Deserialize, Serialize}; -use std::collections::HashSet; use super::harness::memory_context::{WORKING_MEMORY_KEY_PREFIX, WORKING_MEMORY_LIMIT}; @@ -110,49 +109,22 @@ impl MemoryLoader for DefaultMemoryLoader { memory: &dyn Memory, user_message: &str, ) -> anyhow::Result { - let entries = memory - .recall( - user_message, - self.limit, - crate::openhuman::memory::RecallOpts::default(), - ) - .await?; + // Primary `[Memory context]` semantic recall used to be injected here, + // but it duplicated content the agent can already reach via the + // compressed memory tree (eager prefetch) and the on-demand memory + // search tool — and worse, the auto-saved `user_msg` entry would come + // back as the top "relevant" memory and echo the user's text back at + // them. Only the bounded `[User working memory]` block remains: it + // surfaces sync-derived profile facts (timezone, preferences) that the + // tree digest doesn't always carry, and it is keyed by a fixed + // `working.user.*` namespace so it can't catch arbitrary chat content. let mut context = String::new(); let budget = if self.max_context_chars > 0 { self.max_context_chars } else { usize::MAX }; - let mut seen_keys = HashSet::new(); - let header = "[Memory context]\n"; - for entry in entries { - if let Some(score) = entry.score { - if score < self.min_relevance_score { - continue; - } - } - let line = format!("- {}: {}\n", entry.key, entry.content); - if context.is_empty() { - if header.len() >= budget { - return Ok(String::new()); - } - context.push_str(header); - } - if context.len() + line.len() > budget { - tracing::debug!( - budget, - current_len = context.len(), - skipped_line_len = line.len(), - "[memory_loader] context budget reached, skipping remaining entries" - ); - break; - } - seen_keys.insert(entry.key); - context.push_str(&line); - } - - // Explicit bounded recall for sync-derived user working memory. let working_query = format!("working.user {user_message}"); let working_entries = memory .recall( @@ -166,7 +138,6 @@ impl MemoryLoader for DefaultMemoryLoader { for entry in working_entries .into_iter() .filter(|entry| entry.key.starts_with(WORKING_MEMORY_KEY_PREFIX)) - .filter(|entry| !seen_keys.contains(&entry.key)) .filter(|entry| match entry.score { Some(score) => score >= self.min_relevance_score, None => true, @@ -175,7 +146,7 @@ impl MemoryLoader for DefaultMemoryLoader { { if !appended_working_header { let section = "[User working memory]\n"; - if context.len() + section.len() > budget { + if section.len() > budget { break; } context.push_str(section); diff --git a/tests/agent_memory_loader_public.rs b/tests/agent_memory_loader_public.rs index 6d924cfa9..4e95ce1fb 100644 --- a/tests/agent_memory_loader_public.rs +++ b/tests/agent_memory_loader_public.rs @@ -85,7 +85,11 @@ fn entry(key: &str, content: &str, score: Option) -> MemoryEntry { } #[tokio::test] -async fn loader_merges_primary_and_working_memory_with_filters() -> Result<()> { +async fn loader_skips_primary_recall_and_filters_working_memory() -> Result<()> { + // The open-ended `[Memory context]` recall block was removed: it duplicated + // what the memory tree + memory search tool already cover, and would echo + // the just-saved `user_msg` entry back at the user. The loader now only + // emits the bounded `[User working memory]` block. let memory: Arc = Arc::new(ScriptedMemory { primary: vec![ entry("high", "keep me", Some(0.9)), @@ -93,7 +97,7 @@ async fn loader_merges_primary_and_working_memory_with_filters() -> Result<()> { ], working: vec![ entry("working.user.pref", "concise", Some(0.95)), - entry("high", "duplicate", Some(0.95)), + entry("not.working.user", "ignored", Some(0.95)), ], }); @@ -102,12 +106,12 @@ async fn loader_merges_primary_and_working_memory_with_filters() -> Result<()> { .load_context(memory.as_ref(), "hello") .await?; - assert!(context.contains("[Memory context]")); - assert!(context.contains("- high: keep me")); + assert!(!context.contains("[Memory context]")); + assert!(!context.contains("keep me")); assert!(!context.contains("drop me")); assert!(context.contains("[User working memory]")); assert!(context.contains("working.user.pref")); - assert!(!context.contains("duplicate")); + assert!(!context.contains("not.working.user")); Ok(()) } @@ -130,23 +134,30 @@ async fn loader_can_return_only_working_memory_when_primary_is_empty() -> Result #[tokio::test] async fn loader_respects_tight_budgets() -> Result<()> { + // Primary `[Memory context]` recall is no longer injected, so any + // entries on the `primary` channel must be ignored regardless of budget. + // Tight budgets that can't fit the `[User working memory]` header should + // produce an empty context. let memory: Arc = Arc::new(ScriptedMemory { primary: vec![entry("main", "1234567890", Some(0.9))], working: vec![entry("working.user.tip", "include me", Some(0.9))], }); - let header_len = "[Memory context]\n".len(); + let header = "[User working memory]\n"; let empty = DefaultMemoryLoader::new(1, 0.4) - .with_max_chars(header_len) + .with_max_chars(header.len() - 1) .load_context(memory.as_ref(), "hello") .await?; assert!(empty.is_empty()); + let line = "- working.user.tip: include me\n"; let bounded = DefaultMemoryLoader::new(1, 0.4) - .with_max_chars("[Memory context]\n- main: 1234567890\n".len() + 1) + .with_max_chars(header.len() + line.len() + 1) .load_context(memory.as_ref(), "hello") .await?; - assert!(bounded.contains("- main: 1234567890")); - assert!(!bounded.contains("working.user.tip")); + assert!(bounded.contains("[User working memory]")); + assert!(bounded.contains("- working.user.tip: include me")); + // Primary recall is gone — `main` must never appear. + assert!(!bounded.contains("- main: 1234567890")); Ok(()) }