fix(agent): drop redundant [Memory context] recall injection (#1173)

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<String> {
-        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);

tests/agent_memory_loader_public.rs

@@ -85,7 +85,11 @@ fn entry(key: &str, content: &str, score: Option<f64>) -> 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<dyn Memory> = 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<dyn Memory> = 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(())
 }