diff --git a/scripts/bench-memory-retrieval.sh b/scripts/bench-memory-retrieval.sh new file mode 100755 index 000000000..be0fcd945 --- /dev/null +++ b/scripts/bench-memory-retrieval.sh @@ -0,0 +1,193 @@ +#!/usr/bin/env bash +# bench-memory-retrieval.sh — benchmark memory retrieval quality and latency. +# +# Tests the core's memory query (semantic search) and tree file-based content +# retrieval against a set of benchmark queries. Measures wall-clock time per +# query, result count, and content quality. +# +# Usage: +# ./scripts/bench-memory-retrieval.sh +# ./scripts/bench-memory-retrieval.sh --verbose + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" + +VERBOSE=0 +[[ "${1:-}" == "--verbose" ]] && VERBOSE=1 + +CORE_BIN="$REPO_ROOT/target/debug/openhuman-core" +if [[ ! -x "$CORE_BIN" ]]; then + echo "ERROR: Build openhuman-core first: cargo build --bin openhuman-core" + exit 1 +fi + +WORKSPACE_DIR="${OPENHUMAN_WORKSPACE:-$HOME/.openhuman-staging}" +USERS_DIR="$WORKSPACE_DIR/users" + +if [[ ! -d "$USERS_DIR" ]]; then + echo "ERROR: Workspace users dir not found: $USERS_DIR" + exit 1 +fi + +# Find first user workspace with memory_tree content +CONTENT_ROOT=$(find "$USERS_DIR" -path "*/workspace/memory_tree/content" -type d 2>/dev/null | head -1 || true) +if [[ -z "$CONTENT_ROOT" ]]; then + echo "ERROR: No memory_tree content found under $USERS_DIR/" + exit 1 +fi +RESULTS_DIR="$REPO_ROOT/target/bench-memory" +mkdir -p "$RESULTS_DIR" +TIMESTAMP=$(date +%Y%m%d-%H%M%S) +RESULTS_FILE="$RESULTS_DIR/retrieval-$TIMESTAMP.txt" + +# Inventory +FILE_COUNT=$(find "$CONTENT_ROOT" -type f -name "*.md" 2>/dev/null | wc -l | tr -d ' ') +TOTAL_SIZE=$(du -sh "$CONTENT_ROOT" 2>/dev/null | cut -f1) +CHAT_COUNT=$(find "$CONTENT_ROOT/chat" -type f -name "*.md" 2>/dev/null | wc -l | tr -d ' ') +EPISODIC_COUNT=$(find "$CONTENT_ROOT/episodic" -type f -name "*.md" 2>/dev/null | wc -l | tr -d ' ') +RAW_COUNT=$(find "$CONTENT_ROOT/raw" -type f -name "*.md" 2>/dev/null | wc -l | tr -d ' ') +WIKI_COUNT=$(find "$CONTENT_ROOT/wiki" -type f -name "*.md" 2>/dev/null | wc -l | tr -d ' ') + +cat <&1) || true + + END_NS=$(python3 -c "import time; print(int(time.time()*1e9))") + ELAPSED_MS=$(python3 -c "print(($END_NS - $START_NS) / 1_000_000)") + + RESULT_COUNT=$(echo "$OUTPUT" | grep -c '^\[' || true) + FIRST_RESULT=$(echo "$OUTPUT" | grep '^\[' | head -1 | cut -c1-120) + + echo " -> ${ELAPSED_MS}ms, $RESULT_COUNT results" | tee -a "$RESULTS_FILE" + if [[ -n "$FIRST_RESULT" ]]; then + echo " -> Top: $FIRST_RESULT..." | tee -a "$RESULTS_FILE" + fi + + if [[ $VERBOSE -eq 1 ]]; then + echo "$OUTPUT" | head -20 + fi +done + +cat </dev/null | wc -l | tr -d ' ') + SAMPLE=$(grep -rl "$pattern" "$CONTENT_ROOT" 2>/dev/null | head -3 | while read f; do + basename "$f" | tr '\n' ' ' + done) + + END_NS=$(python3 -c "import time; print(int(time.time()*1e9))") + ELAPSED_MS=$(python3 -c "print(($END_NS - $START_NS) / 1_000_000)") + + echo " -> ${ELAPSED_MS}ms, $HIT_COUNT files matched" | tee -a "$RESULTS_FILE" + if [[ -n "$SAMPLE" ]]; then + echo " -> Sample: $SAMPLE" | tee -a "$RESULTS_FILE" + fi +done + +cat <&1) || true + + END_NS=$(python3 -c "import time; print(int(time.time()*1e9))") + ELAPSED_MS=$(python3 -c "print(($END_NS - $START_NS) / 1_000_000)") + + # Extract hit count from JSON + HITS=$(echo "$OUTPUT" | python3 -c " +import sys,json +raw = sys.stdin.read() +last_line = [l for l in raw.split('\n') if '{' in l] +d = json.loads(last_line[-1]) if last_line else {} +print(d.get('result',{}).get('total',0)) +" 2>/dev/null || echo "parse-error") + + echo " -> ${ELAPSED_MS}ms, hits=$HITS" | tee -a "$RESULTS_FILE" + + if [[ $VERBOSE -eq 1 ]]; then + echo "$OUTPUT" | tail -5 + fi +done + +cat </dev/null | wc -l | tr -d ' ') +DIR_COUNT=$(find "$CONTENT_ROOT" -type d 2>/dev/null | wc -l | tr -d ' ') +TOTAL_SIZE=$(du -sh "$CONTENT_ROOT" 2>/dev/null | cut -f1) + +echo "==============================================" +echo " Memory Tree Walk Benchmark" +echo "==============================================" +echo "" +echo "Content root: $CONTENT_ROOT" +echo "Files: $FILE_COUNT markdown files" +echo "Directories: $DIR_COUNT" +echo "Total size: $TOTAL_SIZE" +echo "Max turns: $MAX_TURNS" +echo "Namespace: $NAMESPACE" +if [[ -n "$MODEL" ]]; then + echo "Model: $MODEL" +fi +echo "" + +# Build the queries array +if [[ -n "$CUSTOM_QUERY" ]]; then + QUERIES=("$CUSTOM_QUERY") +else + QUERIES=("${DEFAULT_QUERIES[@]}") +fi + +# Check if the core binary exists +CORE_BIN="$REPO_ROOT/target/debug/openhuman-core" +if [[ ! -x "$CORE_BIN" ]]; then + echo "Building openhuman-core..." + cargo build --manifest-path "$REPO_ROOT/Cargo.toml" --bin openhuman-core 2>&1 | tail -3 + echo "" +fi + +# Results storage +RESULTS_DIR="$REPO_ROOT/target/bench-memory" +mkdir -p "$RESULTS_DIR" +TIMESTAMP=$(date +%Y%m%d-%H%M%S) +RESULTS_FILE="$RESULTS_DIR/bench-$TIMESTAMP.jsonl" + +echo "----------------------------------------------" +echo " Running ${#QUERIES[@]} queries" +echo "----------------------------------------------" +echo "" + +TOTAL_START=$(date +%s%N 2>/dev/null || python3 -c "import time; print(int(time.time()*1e9))") +PASS=0 +FAIL=0 + +for i in "${!QUERIES[@]}"; do + query="${QUERIES[$i]}" + idx=$((i + 1)) + echo "[$idx/${#QUERIES[@]}] $query" + + QUERY_START=$(date +%s%N 2>/dev/null || python3 -c "import time; print(int(time.time()*1e9))") + + # Call the core CLI with the memory_smart_walk RPC + # Use python3 to safely build the JSON payload and avoid query injection + RPC_PAYLOAD=$(python3 -c " +import json, sys +payload = { + 'jsonrpc': '2.0', + 'id': 'bench-$idx', + 'method': 'openhuman.memory_smart_walk', + 'params': { + 'query': sys.argv[1], + 'namespace': sys.argv[2], + 'max_turns': $MAX_TURNS + } +} +print(json.dumps(payload)) +" "$query" "$NAMESPACE") + + # Use the CLI's rpc subcommand if available, otherwise use the tool directly + if [[ $VERBOSE -eq 1 ]]; then + OUTPUT=$("$CORE_BIN" rpc --stdin <<< "$RPC_PAYLOAD" 2>&1) || true + echo "$OUTPUT" + else + OUTPUT=$("$CORE_BIN" rpc --stdin <<< "$RPC_PAYLOAD" 2>/dev/null) || true + fi + + QUERY_END=$(date +%s%N 2>/dev/null || python3 -c "import time; print(int(time.time()*1e9))") + ELAPSED_MS=$(( (QUERY_END - QUERY_START) / 1000000 )) + + if echo "$OUTPUT" | grep -q '"result"'; then + PASS=$((PASS + 1)) + STATUS="OK" + else + FAIL=$((FAIL + 1)) + STATUS="FAIL" + fi + + echo " -> ${STATUS} in ${ELAPSED_MS}ms" + + # Log to JSONL — use python3 to safely encode the query string + python3 -c " +import json, sys +record = { + 'query': sys.argv[1], + 'elapsed_ms': $ELAPSED_MS, + 'status': sys.argv[2], + 'timestamp': sys.argv[3] +} +print(json.dumps(record)) +" "$query" "$STATUS" "$(date -u +%Y-%m-%dT%H:%M:%SZ)" >> "$RESULTS_FILE" + echo "" +done + +TOTAL_END=$(date +%s%N 2>/dev/null || python3 -c "import time; print(int(time.time()*1e9))") +TOTAL_MS=$(( (TOTAL_END - TOTAL_START) / 1000000 )) + +echo "==============================================" +echo " Summary" +echo "==============================================" +echo "" +echo "Total queries: ${#QUERIES[@]}" +echo "Passed: $PASS" +echo "Failed: $FAIL" +echo "Total time: ${TOTAL_MS}ms" +echo "Results saved to: $RESULTS_FILE" +echo "" + +if [[ $FAIL -gt 0 ]]; then + echo "WARNING: $FAIL queries failed. Run with --verbose to see errors." + exit 1 +fi diff --git a/src/openhuman/agent/memory_loader.rs b/src/openhuman/agent/memory_loader.rs index 3cc6319d8..6faa3d29e 100644 --- a/src/openhuman/agent/memory_loader.rs +++ b/src/openhuman/agent/memory_loader.rs @@ -1,893 +1,5 @@ -use std::path::PathBuf; +//! Re-export from `agent_memory::memory_loader` — canonical home moved in +//! the `agent_memory` domain consolidation. Existing `use` paths in the +//! harness continue to work via this facade. -use crate::openhuman::memory::Memory; -use crate::openhuman::util::provenance_tag; -use async_trait::async_trait; -use serde::{Deserialize, Serialize}; - -use super::harness::memory_context::{ - CROSS_CHAT_LIMIT, CROSS_CHAT_SNIPPET_CHARS, WORKING_MEMORY_KEY_PREFIX, WORKING_MEMORY_LIMIT, -}; -use crate::openhuman::learning::transcript_ingest::CONVERSATION_MEMORY_NAMESPACE; -use crate::openhuman::memory_conversations::ConversationStore; - -/// Maximum number of `[Prior conversations]` lines surfaced into the prompt -/// at the start of a fresh chat. Tight cap on purpose: this block is meant -/// to recover continuity for high-importance facts, not to dump session -/// history into context. See issue #1399. -const PRIOR_CONVERSATION_LIMIT: usize = 3; -/// Only the importance prefix `high.` survives into the prompt block. -/// Medium/low entries stay queryable via the on-demand memory tool but -/// do not auto-pollute every fresh chat. -const PRIOR_CONVERSATION_KEY_PREFIX: &str = "high."; - -/// Parse a `MemoryEntry::timestamp` (RFC 3339) into an absolute -/// `YYYY-MM-DD` label for prompt injection, e.g. `2026-05-25`. Returns -/// `None` when the timestamp is missing or unparseable so callers omit -/// the stamp rather than emit a garbage date. -/// -/// Time-sensitive memory ("finish the proposal by Wednesday") is a prime -/// vector for stale-as-current hallucinations: with no date the model -/// can't tell a four-day-old working fact from a present-tense one, so it -/// may serve it as today's — the same failure as the memory-tree path. -/// This block feeds the chat user message *and*, via -/// `last_memory_context`, every typed sub-agent including the cron -/// morning briefing (#2944). Reuses the prompt layer's absolute-date -/// formatter for one consistent date shape across surfaces. -fn memory_entry_date_label(timestamp: &str) -> Option { - chrono::DateTime::parse_from_rfc3339(timestamp) - .ok() - .map(|dt| { - crate::openhuman::agent::prompts::memory_date_label(dt.with_timezone(&chrono::Utc)) - }) -} - -/// Canonical header for the `[Cross-chat context]` block injected on -/// every turn that has FTS-surfaced hits from other threads. -/// -/// The "historical" / "capabilities may have changed since" suffix is -/// deliberate: it tells the model these snippets are snapshots from -/// earlier moments and that capability claims (e.g. "I can't delete -/// emails") may be stale because the tool surface or per-toolkit scope -/// toggles can change between chats. -/// -/// Single source of truth — all three call sites bind to this constant -/// so a wording tweak doesn't drift between (a) `memory_loader.rs`'s -/// primary JSONL path, (b) `harness/memory_context.rs`'s fallback -/// recall path, and (c) the orchestrator's "Capability questions" -/// prompt section that names the header verbatim. Tests assert on this -/// constant too — see `memory_loader::tests` and -/// `harness::memory_context::tests`. -pub const CROSS_CHAT_HEADER: &str = - "[Cross-chat context — historical; capabilities may have changed since]\n"; - -#[async_trait] -pub trait MemoryLoader: Send + Sync { - async fn load_context(&self, memory: &dyn Memory, user_message: &str) - -> anyhow::Result; -} - -pub struct DefaultMemoryLoader { - limit: usize, - min_relevance_score: f64, - /// Maximum characters of memory context to inject (0 = unlimited). - max_context_chars: usize, - /// Workspace dir for direct cross-thread JSONL search (issue #1505). - /// `None` falls back to the Memory-trait recall path. - workspace_dir: Option, -} - -/// Lightweight citation object derived from recalled memory entries. -/// -/// These citations are attached to agent responses so the UI can show -/// provenance for memory-informed answers without exposing full raw memory. -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -pub struct MemoryCitation { - pub id: String, - pub key: String, - #[serde(skip_serializing_if = "Option::is_none")] - pub namespace: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub score: Option, - pub timestamp: String, - pub snippet: String, -} - -impl Default for DefaultMemoryLoader { - fn default() -> Self { - Self { - limit: 5, - min_relevance_score: 0.4, - max_context_chars: 2000, - workspace_dir: None, - } - } -} - -impl DefaultMemoryLoader { - pub fn new(limit: usize, min_relevance_score: f64) -> Self { - Self { - limit: limit.max(1), - min_relevance_score, - max_context_chars: 2000, - workspace_dir: None, - } - } - - pub fn with_max_chars(mut self, max_chars: usize) -> Self { - self.max_context_chars = max_chars; - self - } - - /// Wire the workspace dir so the `[Cross-chat context]` block can do - /// direct JSONL scans across threads (issue #1505). Without this the - /// loader still falls back to the Memory-trait recall path, which only - /// surfaces hits from archived chats (episodic_log). - pub fn with_workspace_dir(mut self, workspace_dir: PathBuf) -> Self { - self.workspace_dir = Some(workspace_dir); - self - } -} - -/// Collect citation metadata from semantic memory recall for a user turn. -/// -/// This mirrors the primary recall path used by `DefaultMemoryLoader` so the -/// UI can display trusted sources whenever memory context influenced a reply. -pub async fn collect_recall_citations( - memory: &dyn Memory, - user_message: &str, - limit: usize, - min_relevance_score: f64, -) -> anyhow::Result> { - let entries = memory - .recall( - user_message, - limit.max(1), - crate::openhuman::memory::RecallOpts::default(), - ) - .await?; - - let citations = entries - .into_iter() - .filter(|entry| match entry.score { - Some(score) => score >= min_relevance_score, - None => true, - }) - .map(|entry| { - let snippet = if entry.content.chars().count() > 280 { - crate::openhuman::util::truncate_with_ellipsis(&entry.content, 280) - } else { - entry.content - }; - MemoryCitation { - id: entry.id, - key: entry.key, - namespace: entry.namespace, - score: entry.score, - timestamp: entry.timestamp, - snippet, - } - }) - .collect(); - - Ok(citations) -} - -#[async_trait] -impl MemoryLoader for DefaultMemoryLoader { - async fn load_context( - &self, - memory: &dyn Memory, - user_message: &str, - ) -> anyhow::Result { - // 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 working_query = format!("working.user {user_message}"); - let working_entries = memory - .recall( - &working_query, - WORKING_MEMORY_LIMIT + 2, - crate::openhuman::memory::RecallOpts::default(), - ) - .await - .unwrap_or_default(); - let mut appended_working_header = false; - for entry in working_entries - .into_iter() - .filter(|entry| entry.key.starts_with(WORKING_MEMORY_KEY_PREFIX)) - .filter(|entry| match entry.score { - Some(score) => score >= self.min_relevance_score, - None => true, - }) - .take(WORKING_MEMORY_LIMIT) - { - if !appended_working_header { - let section = "[User working memory]\n"; - if section.len() > budget { - break; - } - context.push_str(section); - appended_working_header = true; - } - // Stamp each fact with its last-updated date so the model can - // compare against the current date and not present a stale - // working fact as current (#2944). - let line = match memory_entry_date_label(&entry.timestamp) { - Some(date) => format!("- {} (as of {date}): {}\n", entry.key, entry.content), - None => format!("- {}: {}\n", entry.key, entry.content), - }; - if context.len() + line.len() > budget { - tracing::debug!( - budget, - current_len = context.len(), - skipped_line_len = line.len(), - "[memory_loader] context budget reached while appending working memory" - ); - break; - } - context.push_str(&line); - } - - // ── Prior conversations (issue #1399) ───────────────────────── - // High-importance, transcript-derived facts from earlier chats. - // Namespace-scoped recall keeps this block small and tightly - // bounded — only entries the heuristic extractor flagged as - // `high.*` are eligible, and only the first short snippet of - // each is included so the block never crowds out the user's - // actual message. - let prior_query = format!("{} {}", CONVERSATION_MEMORY_NAMESPACE, user_message); - let prior_entries = memory - .recall( - &prior_query, - PRIOR_CONVERSATION_LIMIT * 4, - crate::openhuman::memory::RecallOpts { - namespace: Some(CONVERSATION_MEMORY_NAMESPACE), - ..Default::default() - }, - ) - .await - .unwrap_or_default(); - - let mut appended_prior_header = false; - let mut prior_added = 0usize; - for entry in prior_entries - .into_iter() - .filter(|e| e.key.starts_with(PRIOR_CONVERSATION_KEY_PREFIX)) - .filter(|e| match e.score { - Some(score) => score >= self.min_relevance_score, - None => true, - }) - { - if prior_added >= PRIOR_CONVERSATION_LIMIT { - break; - } - // The stored content is two lines: - // [high preference] I prefer Postgres ... - // [provenance] {"thread_id":"thr_…", ...} - // For the prompt we keep only the first line so the block - // stays compact. Provenance survives in the underlying - // memory entry and is queryable through the memory tool. - let primary = entry - .content - .lines() - .find(|l| !l.trim_start().starts_with("[provenance]")) - .unwrap_or(&entry.content) - .trim(); - if primary.is_empty() { - continue; - } - if !appended_prior_header { - let section = "[Prior conversations]\n"; - if context.len() + section.len() > budget { - break; - } - context.push_str(section); - appended_prior_header = true; - } - // Date-stamp the fact so a months-old "high importance" - // statement isn't read as a present-tense commitment (#2944). - let line = match memory_entry_date_label(&entry.timestamp) { - Some(date) => format!("- (noted {date}) {primary}\n"), - None => format!("- {primary}\n"), - }; - if context.len() + line.len() > budget { - tracing::debug!( - budget, - current_len = context.len(), - skipped_line_len = line.len(), - "[memory_loader] context budget reached while appending prior conversations" - ); - break; - } - context.push_str(&line); - prior_added += 1; - } - - // ── Cross-chat context (#1505) ─────────────────────────────────── - // - // Same user, multiple chats. Primary source: direct JSONL scan - // across `/memory/conversations/threads/*.jsonl` via - // `ConversationStore::search_cross_thread_messages`. JSONL is - // append-per-turn, so cross-chat hits surface immediately — - // unlike the durable-fact pipeline (`learning::transcript_ingest`) - // which is async/batched and the episodic_log archivist path - // which only fires on explicit `archive_session`. - // - // The current chat's `thread_id` (from the channel-side - // `with_thread_id` task-local) is excluded so the block doesn't - // echo same-chat history. - // - // Fallback: when `workspace_dir` is not wired (e.g. tests, or a - // headless run that didn't go through the session builder), call - // through `memory.recall` with `cross_session=true` instead. - // That path reads `episodic_log` (populated only by the - // archivist tool) so it's a best-effort secondary signal. - let current_thread_id = - crate::openhuman::inference::provider::thread_context::current_thread_id(); - let cross_hits: Vec<(String, String)> = if let Some(workspace_dir) = &self.workspace_dir { - let store = ConversationStore::new(workspace_dir.clone()); - match store.search_cross_thread_messages( - user_message, - CROSS_CHAT_LIMIT * 4, - current_thread_id.as_deref(), - ) { - Ok(hits) => { - tracing::debug!( - "[memory_loader] cross-chat JSONL scan returned {} hits (exclude={:?})", - hits.len(), - current_thread_id - ); - hits.into_iter() - .filter(|h| h.score >= self.min_relevance_score) - .take(CROSS_CHAT_LIMIT) - .map(|h| (h.thread_id, h.content)) - .collect() - } - Err(e) => { - tracing::warn!("[memory_loader] cross-chat JSONL scan failed (non-fatal): {e}"); - Vec::new() - } - } - } else { - // Fallback path (no workspace_dir wired) - let cross_session_opts = crate::openhuman::memory::RecallOpts { - session_id: current_thread_id.as_deref(), - cross_session: true, - min_score: Some(self.min_relevance_score), - ..Default::default() - }; - let entries = memory - .recall(user_message, CROSS_CHAT_LIMIT * 3, cross_session_opts) - .await - .unwrap_or_default(); - entries - .into_iter() - .filter(|e| e.id.starts_with("episodic-cross:")) - .filter(|e| { - // Fallback entries may carry a JSON session blob - // (`{"thread_id": "...", "client_id": "..."}`) rather - // than a bare thread_id, so the SQL-side exclusion - // can miss. Re-check on this side using the same - // normalization shape. - let Some(current_tid) = current_thread_id.as_deref() else { - return true; - }; - let Some(raw_sid) = e.session_id.as_deref() else { - return true; - }; - let sid_thread = serde_json::from_str::(raw_sid) - .ok() - .and_then(|v| { - v.get("thread_id") - .and_then(|t| t.as_str().map(|s| s.to_string())) - }) - .unwrap_or_else(|| raw_sid.to_string()); - sid_thread != current_tid - }) - .filter(|e| match e.score { - Some(score) => score >= self.min_relevance_score, - None => true, - }) - .take(CROSS_CHAT_LIMIT) - .map(|e| { - let sid = e - .session_id - .clone() - .unwrap_or_else(|| "unknown".to_string()); - (sid, e.content) - }) - .collect() - }; - - let mut appended_cross_header = false; - for (sid, content) in cross_hits { - let snippet = if content.chars().count() > CROSS_CHAT_SNIPPET_CHARS { - crate::openhuman::util::truncate_with_ellipsis(&content, CROSS_CHAT_SNIPPET_CHARS) - } else { - content - }; - let prov = provenance_tag(&sid); - if !appended_cross_header { - // The header explicitly labels these snippets as historical so - // the model down-weights them when answering capability - // questions — see CROSS_CHAT_HEADER doc for the rationale and - // the cross-module wording contract. - if context.len() + CROSS_CHAT_HEADER.len() > budget { - break; - } - context.push_str(CROSS_CHAT_HEADER); - appended_cross_header = true; - } - let line = format!("- [{prov}] {snippet}\n"); - if context.len() + line.len() > budget { - tracing::debug!( - budget, - current_len = context.len(), - skipped_line_len = line.len(), - "[memory_loader] context budget reached while appending cross-chat context" - ); - break; - } - context.push_str(&line); - } - - if context.is_empty() { - return Ok(String::new()); - } - context.push('\n'); - Ok(context) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::openhuman::memory::{Memory, MemoryCategory, MemoryEntry}; - - struct MockMemory { - entries: Vec, - cross_chat: Vec, - } - - impl MockMemory { - fn new(entries: Vec) -> Self { - Self { - entries, - cross_chat: Vec::new(), - } - } - } - - #[async_trait] - impl Memory for MockMemory { - fn name(&self) -> &str { - "mock" - } - - async fn store( - &self, - _namespace: &str, - _key: &str, - _content: &str, - _category: MemoryCategory, - _session_id: Option<&str>, - ) -> anyhow::Result<()> { - Ok(()) - } - - async fn recall( - &self, - _query: &str, - _limit: usize, - opts: crate::openhuman::memory::RecallOpts<'_>, - ) -> anyhow::Result> { - if opts.cross_session { - return Ok(self.cross_chat.clone()); - } - Ok(self.entries.clone()) - } - - async fn get(&self, _namespace: &str, _key: &str) -> anyhow::Result> { - Ok(None) - } - - async fn list( - &self, - _namespace: Option<&str>, - _category: Option<&MemoryCategory>, - _session_id: Option<&str>, - ) -> anyhow::Result> { - Ok(Vec::new()) - } - - async fn forget(&self, _namespace: &str, _key: &str) -> anyhow::Result { - Ok(false) - } - - async fn namespace_summaries( - &self, - ) -> anyhow::Result> { - Ok(Vec::new()) - } - - async fn count(&self) -> anyhow::Result { - Ok(self.entries.len()) - } - - async fn health_check(&self) -> bool { - true - } - } - - fn entry(key: &str, content: &str, score: Option) -> MemoryEntry { - MemoryEntry { - id: format!("id-{key}"), - key: key.to_string(), - content: content.to_string(), - namespace: Some("test".to_string()), - category: MemoryCategory::Conversation, - timestamp: "2026-04-22T00:00:00Z".to_string(), - session_id: None, - score, - taint: Default::default(), - } - } - - #[test] - fn memory_entry_date_label_parses_rfc3339_else_none() { - assert_eq!( - super::memory_entry_date_label("2026-05-25T07:00:00Z").as_deref(), - Some("2026-05-25") - ); - assert_eq!(super::memory_entry_date_label("not-a-date"), None); - assert_eq!(super::memory_entry_date_label(""), None); - } - - #[tokio::test] - async fn loader_stamps_working_memory_with_date() { - // #2944: working-memory facts must carry their last-updated date so - // the model (and downstream sub-agents / the cron briefing, which - // inherit this block) can tell a stale fact from a current one. - let mem = MockMemory::new(vec![MemoryEntry { - id: "id-tz".into(), - key: "working.user.commitment".into(), - content: "Finish the proposal by Wednesday.".into(), - namespace: Some("test".into()), - category: MemoryCategory::Conversation, - timestamp: "2026-05-25T00:00:00Z".into(), - session_id: None, - score: None, - taint: Default::default(), - }]); - - let out = DefaultMemoryLoader::default() - .load_context(&mem, "what's on my plate?") - .await - .expect("loader must succeed"); - - assert!( - out.contains("[User working memory]"), - "expected working-memory block, got:\n{out}" - ); - assert!( - out.contains("(as of 2026-05-25)"), - "working-memory fact must carry its date (#2944), got:\n{out}" - ); - } - - #[tokio::test] - async fn loader_stamps_prior_conversation_with_date() { - // #2944: high-importance prior-chat facts must be dated so a - // months-old statement isn't read as a present-tense commitment. - let mem = MockMemory::new(vec![MemoryEntry { - id: "id-1".into(), - key: "high.preference.aaaaaaaaaaaa".into(), - content: "[high preference] I prefer Postgres for new services.".into(), - namespace: Some(super::CONVERSATION_MEMORY_NAMESPACE.to_string()), - category: MemoryCategory::Conversation, - timestamp: "2026-04-22T00:00:00Z".into(), - session_id: Some("thr_old".into()), - score: Some(0.9), - taint: Default::default(), - }]); - - let out = DefaultMemoryLoader::default() - .load_context(&mem, "what should I default to for storage?") - .await - .expect("loader must succeed"); - - assert!( - out.contains("[Prior conversations]"), - "expected prior conversations block, got:\n{out}" - ); - assert!( - out.contains("(noted 2026-04-22)"), - "prior-conversation fact must carry its date (#2944), got:\n{out}" - ); - } - - #[tokio::test] - async fn loader_surfaces_prior_conversation_high_importance_only() { - // Prior chat extracted two memories: one high-importance preference - // and one medium-importance unresolved task. Only the high one - // should make it into the loader's prompt block (#1399). - let mem = MockMemory::new(vec![ - MemoryEntry { - id: "id-1".into(), - key: "high.preference.aaaaaaaaaaaa".into(), - content: "[high preference] I prefer Postgres for new services.\n[provenance] {\"thread_id\":\"thr_old\"}".into(), - namespace: Some(super::CONVERSATION_MEMORY_NAMESPACE.to_string()), - category: MemoryCategory::Conversation, - timestamp: "2026-04-22T00:00:00Z".into(), - session_id: Some("thr_old".into()), - score: Some(0.9), - taint: Default::default(), - }, - MemoryEntry { - id: "id-2".into(), - key: "med.unresolved_task.bbbbbbbbbbbb".into(), - content: "[med unresolved_task] still need to migrate auth.".into(), - namespace: Some(super::CONVERSATION_MEMORY_NAMESPACE.to_string()), - category: MemoryCategory::Conversation, - timestamp: "2026-04-22T00:00:00Z".into(), - session_id: None, - score: Some(0.9), - taint: Default::default(), - }, - ]); - - let loader = DefaultMemoryLoader::default(); - let out = loader - .load_context(&mem, "what should I default to for storage?") - .await - .expect("loader must succeed"); - - assert!( - out.contains("[Prior conversations]"), - "expected prior conversations block, got:\n{out}" - ); - assert!(out.contains("Postgres")); - assert!( - !out.contains("migrate auth"), - "med-importance entries must not auto-surface, got:\n{out}" - ); - assert!( - !out.contains("[provenance]"), - "provenance is not rendered into the prompt block, got:\n{out}" - ); - } - - #[tokio::test] - async fn collect_recall_citations_filters_and_truncates_entries() { - let mem = MockMemory::new(vec![ - entry("keep", "useful context", Some(0.9)), - entry("drop", "too weak", Some(0.1)), - entry("long", &"x".repeat(600), Some(0.8)), - ]); - - let citations = collect_recall_citations(&mem, "hello", 5, 0.4) - .await - .expect("citation collection should succeed"); - assert_eq!(citations.len(), 2); - assert_eq!(citations[0].key, "keep"); - assert_eq!(citations[1].key, "long"); - assert!(citations[1].snippet.ends_with("...")); - } - - // ── Cross-chat context (#1505) ─────────────────────────────────────── - - fn cross_chat_entry( - cross_id: &str, - session_id: &str, - content: &str, - score: Option, - ) -> MemoryEntry { - MemoryEntry { - id: format!("episodic-cross:{cross_id}"), - key: format!("{session_id}:user"), - content: content.into(), - namespace: None, - category: MemoryCategory::Conversation, - timestamp: "2026-05-15T00:00:00Z".into(), - session_id: Some(session_id.into()), - score, - taint: Default::default(), - } - } - - #[tokio::test] - async fn loader_surfaces_cross_chat_block_with_provenance_tag() { - let mut mem = MockMemory::new(Vec::new()); - mem.cross_chat = vec![cross_chat_entry( - "1", - "thread-source", - "I prefer Postgres for new services", - Some(0.9), - )]; - - let loader = DefaultMemoryLoader::default(); - let out = loader - .load_context(&mem, "what database should I use?") - .await - .expect("loader must succeed"); - assert!( - out.contains(CROSS_CHAT_HEADER.trim_end()), - "expected cross-chat header, got:\n{out}" - ); - assert!( - out.contains("Postgres"), - "expected the cross-chat fact in the loader output, got:\n{out}" - ); - assert!( - out.contains("[chat:"), - "expected provenance tag, got:\n{out}" - ); - assert!( - !out.contains("thread-source"), - "raw session id MUST NOT leak into the prompt — render only the hashed tag, got:\n{out}" - ); - } - - #[tokio::test] - async fn loader_caps_cross_chat_block_at_limit() { - let mut mem = MockMemory::new(Vec::new()); - mem.cross_chat = (0..10) - .map(|i| { - cross_chat_entry( - &format!("{i}"), - &format!("thread-{i}"), - &format!("Cross-chat fact #{i}"), - Some(0.9), - ) - }) - .collect(); - - let loader = DefaultMemoryLoader::default(); - let out = loader - .load_context(&mem, "Cross-chat fact") - .await - .expect("loader must succeed"); - let cross_lines = out.lines().filter(|l| l.starts_with("- [chat:")).count(); - assert!( - cross_lines <= CROSS_CHAT_LIMIT, - "loader cross-chat block must be capped at {CROSS_CHAT_LIMIT}, saw {cross_lines}" - ); - } - - #[tokio::test] - async fn loader_drops_cross_chat_below_relevance_threshold() { - let mut mem = MockMemory::new(Vec::new()); - mem.cross_chat = vec![ - cross_chat_entry("1", "thread-a", "low score chat fact", Some(0.05)), - cross_chat_entry("2", "thread-b", "high score chat fact", Some(0.9)), - ]; - - let loader = DefaultMemoryLoader::default(); - let out = loader - .load_context(&mem, "fact") - .await - .expect("loader must succeed"); - assert!( - out.contains("high score chat fact"), - "high-relevance cross-chat must surface, got:\n{out}" - ); - assert!( - !out.contains("low score chat fact"), - "low-relevance cross-chat must be filtered, got:\n{out}" - ); - } - - #[tokio::test] - async fn loader_returns_empty_when_no_cross_chat_or_other_blocks_match() { - let mem = MockMemory::new(Vec::new()); - let loader = DefaultMemoryLoader::default(); - let out = loader - .load_context(&mem, "anything") - .await - .expect("loader must succeed"); - assert!( - !out.contains(CROSS_CHAT_HEADER.trim_end()), - "no cross-chat hits must produce no header, got:\n{out}" - ); - } - - /// Exercises the **primary** cross-chat path (JSONL scan via - /// `ConversationStore`, not the `Memory::recall` fallback). Writes - /// two threads through `ConversationStore`, wires `workspace_dir` - /// into the loader, and asserts the prompt picks up the hit from - /// the inactive thread with a redacted provenance tag. - /// - /// Production-critical because the fallback `MockMemory` path is - /// what the other loader tests cover — this is the one users - /// actually run. - #[tokio::test] - async fn loader_surfaces_jsonl_primary_path_with_workspace_dir() { - use crate::openhuman::memory_conversations::{ - ConversationMessage, ConversationStore, CreateConversationThread, - }; - - let temp = tempfile::TempDir::new().expect("tempdir"); - let store = ConversationStore::new(temp.path().to_path_buf()); - - // Chat A — durable fact lives here. - store - .ensure_thread(CreateConversationThread { - parent_thread_id: None, - id: "thread-a".to_string(), - title: "Chat A".to_string(), - created_at: "2026-04-10T12:00:00Z".to_string(), - labels: None, - personality_id: None, - }) - .expect("ensure thread-a"); - store - .append_message( - "thread-a", - ConversationMessage { - id: "m-a-1".to_string(), - content: "Remember: my project Phoenix uses Go and PostgreSQL.".to_string(), - message_type: "text".to_string(), - extra_metadata: serde_json::json!({}), - sender: "user".to_string(), - created_at: "2026-04-10T12:01:00Z".to_string(), - }, - ) - .expect("append a"); - - // Chat B — active chat (excluded by current_thread_id wiring is - // not exercised here; we just verify the JSONL path surfaces - // hits from other threads). - store - .ensure_thread(CreateConversationThread { - parent_thread_id: None, - id: "thread-b".to_string(), - title: "Chat B".to_string(), - created_at: "2026-04-10T13:00:00Z".to_string(), - labels: None, - personality_id: None, - }) - .expect("ensure thread-b"); - - // MockMemory's cross_chat list is empty — if the loader fell - // back to the Memory::recall path we'd render nothing. Forcing - // a JSONL primary hit proves the workspace_dir branch ran. - let mem = MockMemory::new(Vec::new()); - let loader = DefaultMemoryLoader::new(5, 0.4).with_workspace_dir(temp.path().to_path_buf()); - - let out = loader - .load_context(&mem, "What database does my project Phoenix use") - .await - .expect("loader must succeed"); - - assert!( - out.contains(CROSS_CHAT_HEADER.trim_end()), - "JSONL primary path must emit the cross-chat header, got:\n{out}" - ); - assert!( - out.contains("PostgreSQL"), - "cross-chat block must carry the matched snippet, got:\n{out}" - ); - assert!( - out.contains("chat:"), - "cross-chat block must render a `chat:` provenance tag, got:\n{out}" - ); - assert!( - !out.contains("thread-a"), - "raw thread_id must not leak into the prompt, got:\n{out}" - ); - } -} +pub use crate::openhuman::agent_memory::memory_loader::*; diff --git a/src/openhuman/agent_memory/README.md b/src/openhuman/agent_memory/README.md new file mode 100644 index 000000000..c47f56a98 --- /dev/null +++ b/src/openhuman/agent_memory/README.md @@ -0,0 +1,68 @@ +# agent_memory + +Memory agent domain — owns the retrieval-focused memory agent, its prompt, and performance instrumentation for memory tree walking and chunk retrieval. + +## Purpose + +The memory agent is a specialist sub-agent that navigates the user's memory tree to answer questions. It combines multiple retrieval strategies: + +1. **Vector search** — semantic similarity across all stored embeddings +2. **Keyword search** — pattern matching across raw content files on disk +3. **Entity search** — canonical entity lookup and relationship following +4. **Tree browse** — hierarchical navigation of time-based summary trees +5. **Content read** — direct file reads from raw/wiki/episodic/document stores +6. **Source listing** — discovery of available sources and content types + +## Module layout + +| File | Role | +|------|------| +| `mod.rs` | Module declarations and re-exports | +| `types.rs` | Benchmark and performance tracking types | +| `ops.rs` | Benchmarking harness for memory walk performance | +| `tools.rs` | `call_memory_agent` tool implementation | + +## Memory tree structure + +The memory tree lives at `{workspace}/memory_tree/content/` with this layout: + +```text +content/ +├── chat/ # Conversation chunks (by source) +│ └── conversations-agent/ +│ └── {hash}.md +├── episodic/ # Session/subconscious episode chunks +│ └── {session_id}/ +│ └── {hash}.md +├── raw/ # Raw ingested documents (GitHub, Gmail, etc.) +│ └── {source-slug}/ +│ └── {hash}.md +└── wiki/ # Summary tree (hierarchical) + └── summaries/ + └── {namespace}/ + └── {level}/{node_id}.md +``` + +## Benchmarking + +Use the benchmark script to measure retrieval performance: + +```bash +# Run default benchmark queries against the staging memory tree +./scripts/bench-memory-walk.sh + +# Custom queries +./scripts/bench-memory-walk.sh --query "what did I discuss about OpenHuman?" --max-turns 15 + +# Custom content root +./scripts/bench-memory-walk.sh --content-root /path/to/memory_tree/content +``` + +## Agent definition + +The built-in agent is registered at `src/openhuman/agent_memory/agent/`: +- `agent.toml` — tool allowlist, model hint, iteration cap +- `prompt.rs` — dynamic prompt builder +- `prompt.md` — system prompt archetype + +The agent has access to the full memory retrieval tool surface: `memory_tree`, `memory_recall`, `memory_smart_walk`, `memory_tree_walk`, plus entity and source querying tools. diff --git a/src/openhuman/agent_memory/agent/agent.toml b/src/openhuman/agent_memory/agent/agent.toml new file mode 100644 index 000000000..cdc89a475 --- /dev/null +++ b/src/openhuman/agent_memory/agent/agent.toml @@ -0,0 +1,28 @@ +id = "agent_memory" +display_name = "Memory Agent" +delegate_name = "retrieve_memory" +when_to_use = "Memory retrieval and tree walking specialist — searches, navigates, and retrieves information from the user's memory tree using vector search, keyword matching, entity lookup, and hierarchical tree browsing. Use when the user asks to find, recall, search, or look up something from their memory, conversations, documents, or knowledge base." +temperature = 0.2 +max_iterations = 15 +sandbox_mode = "read_only" +agent_tier = "worker" +omit_identity = true +omit_memory_context = false +omit_safety_preamble = true +omit_skills_catalog = true +omit_profile = false +omit_memory_md = false + +[model] +hint = "chat" + +[tools] +named = [ + "memory_recall", + "memory_tree", + "memory_tree_walk", + "memory_smart_walk", + "query_memory", + "memory_doctor", + "ask_user_clarification", +] diff --git a/src/openhuman/agent_memory/agent/mod.rs b/src/openhuman/agent_memory/agent/mod.rs new file mode 100644 index 000000000..8bf84783c --- /dev/null +++ b/src/openhuman/agent_memory/agent/mod.rs @@ -0,0 +1 @@ +pub mod prompt; diff --git a/src/openhuman/agent_memory/agent/prompt.md b/src/openhuman/agent_memory/agent/prompt.md new file mode 100644 index 000000000..7de2fa39f --- /dev/null +++ b/src/openhuman/agent_memory/agent/prompt.md @@ -0,0 +1,40 @@ +# Memory Agent + +You are a memory retrieval specialist. Your job is to find and return relevant information from the user's memory tree — conversations, documents, episodic memories, and knowledge base entries. + +## Retrieval strategy + +Use the right tool for the job: + +1. **`memory_smart_walk`** — your primary tool. Combines vector search, keyword matching, entity lookup, and tree browsing. Use for open-ended queries ("what do I know about X?", "find conversations about Y"). +2. **`memory_tree`** — unified dispatcher with modes: + - `search_entities` — find canonical entity IDs first (call before filtering by entity) + - `query_source` — filter by source kind (chat, email, document) + time window + - `drill_down` — expand a summary node one level deeper + - `fetch_leaves` — pull raw chunks for citation +3. **`memory_tree_walk`** — basic tree navigation. Use when you need to explore the hierarchical summary structure step by step. +4. **`memory_recall`** — legacy key-value memory search. Good for exact preference/fact lookups. +5. **`query_memory`** — simple text search across stored memories. +6. **`memory_doctor`** — diagnose tree health issues. + +## Performance contract + +- Start broad, then narrow. Use `search_entities` or `memory_smart_walk` first, then drill down. +- Avoid redundant walks. If `memory_smart_walk` already found the answer, don't re-walk with `memory_tree_walk`. +- Cite sources. Every fact in your answer should trace back to a specific chunk or summary node. +- Report what you didn't find. If the memory tree has gaps, say so explicitly rather than guessing. +- Prefer fewer turns. A 3-turn retrieval is better than an 8-turn one if both reach the same answer. + +## Output format + +Return a clear answer with inline citations. After the answer, list the evidence sources: + +``` +[Answer text with citations like [1], [2]...] + +Sources: +1. chat/conversations-agent/abc123.md — "relevant snippet" +2. raw/github-repo/def456.md — "relevant snippet" +``` + +If the query has no matches, say so directly. Do not fabricate memories. diff --git a/src/openhuman/agent_memory/agent/prompt.rs b/src/openhuman/agent_memory/agent/prompt.rs new file mode 100644 index 000000000..9533fda3e --- /dev/null +++ b/src/openhuman/agent_memory/agent/prompt.rs @@ -0,0 +1,38 @@ +//! System prompt builder for the `agent_memory` built-in agent. + +use crate::openhuman::context::prompt::{ + render_safety, render_tools, render_user_files, render_workspace, PromptContext, +}; +use anyhow::Result; + +const ARCHETYPE: &str = include_str!("prompt.md"); + +pub fn build(ctx: &PromptContext<'_>) -> Result { + let mut out = String::with_capacity(4096); + out.push_str(ARCHETYPE.trim_end()); + out.push_str("\n\n"); + + let user_files = render_user_files(ctx)?; + if !user_files.trim().is_empty() { + out.push_str(user_files.trim_end()); + out.push_str("\n\n"); + } + + let tools = render_tools(ctx)?; + if !tools.trim().is_empty() { + out.push_str(tools.trim_end()); + out.push_str("\n\n"); + } + + let safety = render_safety(); + out.push_str(safety.trim_end()); + out.push_str("\n\n"); + + let workspace = render_workspace(ctx)?; + if !workspace.trim().is_empty() { + out.push_str(workspace.trim_end()); + out.push('\n'); + } + + Ok(out) +} diff --git a/src/openhuman/agent_memory/memory_loader.rs b/src/openhuman/agent_memory/memory_loader.rs new file mode 100644 index 000000000..88564b94d --- /dev/null +++ b/src/openhuman/agent_memory/memory_loader.rs @@ -0,0 +1,893 @@ +use std::path::PathBuf; + +use crate::openhuman::memory::Memory; +use crate::openhuman::util::provenance_tag; +use async_trait::async_trait; +use serde::{Deserialize, Serialize}; + +use crate::openhuman::agent::harness::memory_context::{ + CROSS_CHAT_LIMIT, CROSS_CHAT_SNIPPET_CHARS, WORKING_MEMORY_KEY_PREFIX, WORKING_MEMORY_LIMIT, +}; +use crate::openhuman::learning::transcript_ingest::CONVERSATION_MEMORY_NAMESPACE; +use crate::openhuman::memory_conversations::ConversationStore; + +/// Maximum number of `[Prior conversations]` lines surfaced into the prompt +/// at the start of a fresh chat. Tight cap on purpose: this block is meant +/// to recover continuity for high-importance facts, not to dump session +/// history into context. See issue #1399. +const PRIOR_CONVERSATION_LIMIT: usize = 3; +/// Only the importance prefix `high.` survives into the prompt block. +/// Medium/low entries stay queryable via the on-demand memory tool but +/// do not auto-pollute every fresh chat. +const PRIOR_CONVERSATION_KEY_PREFIX: &str = "high."; + +/// Parse a `MemoryEntry::timestamp` (RFC 3339) into an absolute +/// `YYYY-MM-DD` label for prompt injection, e.g. `2026-05-25`. Returns +/// `None` when the timestamp is missing or unparseable so callers omit +/// the stamp rather than emit a garbage date. +/// +/// Time-sensitive memory ("finish the proposal by Wednesday") is a prime +/// vector for stale-as-current hallucinations: with no date the model +/// can't tell a four-day-old working fact from a present-tense one, so it +/// may serve it as today's — the same failure as the memory-tree path. +/// This block feeds the chat user message *and*, via +/// `last_memory_context`, every typed sub-agent including the cron +/// morning briefing (#2944). Reuses the prompt layer's absolute-date +/// formatter for one consistent date shape across surfaces. +fn memory_entry_date_label(timestamp: &str) -> Option { + chrono::DateTime::parse_from_rfc3339(timestamp) + .ok() + .map(|dt| { + crate::openhuman::agent::prompts::memory_date_label(dt.with_timezone(&chrono::Utc)) + }) +} + +/// Canonical header for the `[Cross-chat context]` block injected on +/// every turn that has FTS-surfaced hits from other threads. +/// +/// The "historical" / "capabilities may have changed since" suffix is +/// deliberate: it tells the model these snippets are snapshots from +/// earlier moments and that capability claims (e.g. "I can't delete +/// emails") may be stale because the tool surface or per-toolkit scope +/// toggles can change between chats. +/// +/// Single source of truth — all three call sites bind to this constant +/// so a wording tweak doesn't drift between (a) `memory_loader.rs`'s +/// primary JSONL path, (b) `harness/memory_context.rs`'s fallback +/// recall path, and (c) the orchestrator's "Capability questions" +/// prompt section that names the header verbatim. Tests assert on this +/// constant too — see `memory_loader::tests` and +/// `harness::memory_context::tests`. +pub const CROSS_CHAT_HEADER: &str = + "[Cross-chat context — historical; capabilities may have changed since]\n"; + +#[async_trait] +pub trait MemoryLoader: Send + Sync { + async fn load_context(&self, memory: &dyn Memory, user_message: &str) + -> anyhow::Result; +} + +pub struct DefaultMemoryLoader { + limit: usize, + min_relevance_score: f64, + /// Maximum characters of memory context to inject (0 = unlimited). + max_context_chars: usize, + /// Workspace dir for direct cross-thread JSONL search (issue #1505). + /// `None` falls back to the Memory-trait recall path. + workspace_dir: Option, +} + +/// Lightweight citation object derived from recalled memory entries. +/// +/// These citations are attached to agent responses so the UI can show +/// provenance for memory-informed answers without exposing full raw memory. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct MemoryCitation { + pub id: String, + pub key: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub namespace: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub score: Option, + pub timestamp: String, + pub snippet: String, +} + +impl Default for DefaultMemoryLoader { + fn default() -> Self { + Self { + limit: 5, + min_relevance_score: 0.4, + max_context_chars: 2000, + workspace_dir: None, + } + } +} + +impl DefaultMemoryLoader { + pub fn new(limit: usize, min_relevance_score: f64) -> Self { + Self { + limit: limit.max(1), + min_relevance_score, + max_context_chars: 2000, + workspace_dir: None, + } + } + + pub fn with_max_chars(mut self, max_chars: usize) -> Self { + self.max_context_chars = max_chars; + self + } + + /// Wire the workspace dir so the `[Cross-chat context]` block can do + /// direct JSONL scans across threads (issue #1505). Without this the + /// loader still falls back to the Memory-trait recall path, which only + /// surfaces hits from archived chats (episodic_log). + pub fn with_workspace_dir(mut self, workspace_dir: PathBuf) -> Self { + self.workspace_dir = Some(workspace_dir); + self + } +} + +/// Collect citation metadata from semantic memory recall for a user turn. +/// +/// This mirrors the primary recall path used by `DefaultMemoryLoader` so the +/// UI can display trusted sources whenever memory context influenced a reply. +pub async fn collect_recall_citations( + memory: &dyn Memory, + user_message: &str, + limit: usize, + min_relevance_score: f64, +) -> anyhow::Result> { + let entries = memory + .recall( + user_message, + limit.max(1), + crate::openhuman::memory::RecallOpts::default(), + ) + .await?; + + let citations = entries + .into_iter() + .filter(|entry| match entry.score { + Some(score) => score >= min_relevance_score, + None => true, + }) + .map(|entry| { + let snippet = if entry.content.chars().count() > 280 { + crate::openhuman::util::truncate_with_ellipsis(&entry.content, 280) + } else { + entry.content + }; + MemoryCitation { + id: entry.id, + key: entry.key, + namespace: entry.namespace, + score: entry.score, + timestamp: entry.timestamp, + snippet, + } + }) + .collect(); + + Ok(citations) +} + +#[async_trait] +impl MemoryLoader for DefaultMemoryLoader { + async fn load_context( + &self, + memory: &dyn Memory, + user_message: &str, + ) -> anyhow::Result { + // 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 working_query = format!("working.user {user_message}"); + let working_entries = memory + .recall( + &working_query, + WORKING_MEMORY_LIMIT + 2, + crate::openhuman::memory::RecallOpts::default(), + ) + .await + .unwrap_or_default(); + let mut appended_working_header = false; + for entry in working_entries + .into_iter() + .filter(|entry| entry.key.starts_with(WORKING_MEMORY_KEY_PREFIX)) + .filter(|entry| match entry.score { + Some(score) => score >= self.min_relevance_score, + None => true, + }) + .take(WORKING_MEMORY_LIMIT) + { + if !appended_working_header { + let section = "[User working memory]\n"; + if section.len() > budget { + break; + } + context.push_str(section); + appended_working_header = true; + } + // Stamp each fact with its last-updated date so the model can + // compare against the current date and not present a stale + // working fact as current (#2944). + let line = match memory_entry_date_label(&entry.timestamp) { + Some(date) => format!("- {} (as of {date}): {}\n", entry.key, entry.content), + None => format!("- {}: {}\n", entry.key, entry.content), + }; + if context.len() + line.len() > budget { + tracing::debug!( + budget, + current_len = context.len(), + skipped_line_len = line.len(), + "[memory_loader] context budget reached while appending working memory" + ); + break; + } + context.push_str(&line); + } + + // ── Prior conversations (issue #1399) ───────────────────────── + // High-importance, transcript-derived facts from earlier chats. + // Namespace-scoped recall keeps this block small and tightly + // bounded — only entries the heuristic extractor flagged as + // `high.*` are eligible, and only the first short snippet of + // each is included so the block never crowds out the user's + // actual message. + let prior_query = format!("{} {}", CONVERSATION_MEMORY_NAMESPACE, user_message); + let prior_entries = memory + .recall( + &prior_query, + PRIOR_CONVERSATION_LIMIT * 4, + crate::openhuman::memory::RecallOpts { + namespace: Some(CONVERSATION_MEMORY_NAMESPACE), + ..Default::default() + }, + ) + .await + .unwrap_or_default(); + + let mut appended_prior_header = false; + let mut prior_added = 0usize; + for entry in prior_entries + .into_iter() + .filter(|e| e.key.starts_with(PRIOR_CONVERSATION_KEY_PREFIX)) + .filter(|e| match e.score { + Some(score) => score >= self.min_relevance_score, + None => true, + }) + { + if prior_added >= PRIOR_CONVERSATION_LIMIT { + break; + } + // The stored content is two lines: + // [high preference] I prefer Postgres ... + // [provenance] {"thread_id":"thr_…", ...} + // For the prompt we keep only the first line so the block + // stays compact. Provenance survives in the underlying + // memory entry and is queryable through the memory tool. + let primary = entry + .content + .lines() + .find(|l| !l.trim_start().starts_with("[provenance]")) + .unwrap_or(&entry.content) + .trim(); + if primary.is_empty() { + continue; + } + if !appended_prior_header { + let section = "[Prior conversations]\n"; + if context.len() + section.len() > budget { + break; + } + context.push_str(section); + appended_prior_header = true; + } + // Date-stamp the fact so a months-old "high importance" + // statement isn't read as a present-tense commitment (#2944). + let line = match memory_entry_date_label(&entry.timestamp) { + Some(date) => format!("- (noted {date}) {primary}\n"), + None => format!("- {primary}\n"), + }; + if context.len() + line.len() > budget { + tracing::debug!( + budget, + current_len = context.len(), + skipped_line_len = line.len(), + "[memory_loader] context budget reached while appending prior conversations" + ); + break; + } + context.push_str(&line); + prior_added += 1; + } + + // ── Cross-chat context (#1505) ─────────────────────────────────── + // + // Same user, multiple chats. Primary source: direct JSONL scan + // across `/memory/conversations/threads/*.jsonl` via + // `ConversationStore::search_cross_thread_messages`. JSONL is + // append-per-turn, so cross-chat hits surface immediately — + // unlike the durable-fact pipeline (`learning::transcript_ingest`) + // which is async/batched and the episodic_log archivist path + // which only fires on explicit `archive_session`. + // + // The current chat's `thread_id` (from the channel-side + // `with_thread_id` task-local) is excluded so the block doesn't + // echo same-chat history. + // + // Fallback: when `workspace_dir` is not wired (e.g. tests, or a + // headless run that didn't go through the session builder), call + // through `memory.recall` with `cross_session=true` instead. + // That path reads `episodic_log` (populated only by the + // archivist tool) so it's a best-effort secondary signal. + let current_thread_id = + crate::openhuman::inference::provider::thread_context::current_thread_id(); + let cross_hits: Vec<(String, String)> = if let Some(workspace_dir) = &self.workspace_dir { + let store = ConversationStore::new(workspace_dir.clone()); + match store.search_cross_thread_messages( + user_message, + CROSS_CHAT_LIMIT * 4, + current_thread_id.as_deref(), + ) { + Ok(hits) => { + tracing::debug!( + "[memory_loader] cross-chat JSONL scan returned {} hits (exclude={:?})", + hits.len(), + current_thread_id + ); + hits.into_iter() + .filter(|h| h.score >= self.min_relevance_score) + .take(CROSS_CHAT_LIMIT) + .map(|h| (h.thread_id, h.content)) + .collect() + } + Err(e) => { + tracing::warn!("[memory_loader] cross-chat JSONL scan failed (non-fatal): {e}"); + Vec::new() + } + } + } else { + // Fallback path (no workspace_dir wired) + let cross_session_opts = crate::openhuman::memory::RecallOpts { + session_id: current_thread_id.as_deref(), + cross_session: true, + min_score: Some(self.min_relevance_score), + ..Default::default() + }; + let entries = memory + .recall(user_message, CROSS_CHAT_LIMIT * 3, cross_session_opts) + .await + .unwrap_or_default(); + entries + .into_iter() + .filter(|e| e.id.starts_with("episodic-cross:")) + .filter(|e| { + // Fallback entries may carry a JSON session blob + // (`{"thread_id": "...", "client_id": "..."}`) rather + // than a bare thread_id, so the SQL-side exclusion + // can miss. Re-check on this side using the same + // normalization shape. + let Some(current_tid) = current_thread_id.as_deref() else { + return true; + }; + let Some(raw_sid) = e.session_id.as_deref() else { + return true; + }; + let sid_thread = serde_json::from_str::(raw_sid) + .ok() + .and_then(|v| { + v.get("thread_id") + .and_then(|t| t.as_str().map(|s| s.to_string())) + }) + .unwrap_or_else(|| raw_sid.to_string()); + sid_thread != current_tid + }) + .filter(|e| match e.score { + Some(score) => score >= self.min_relevance_score, + None => true, + }) + .take(CROSS_CHAT_LIMIT) + .map(|e| { + let sid = e + .session_id + .clone() + .unwrap_or_else(|| "unknown".to_string()); + (sid, e.content) + }) + .collect() + }; + + let mut appended_cross_header = false; + for (sid, content) in cross_hits { + let snippet = if content.chars().count() > CROSS_CHAT_SNIPPET_CHARS { + crate::openhuman::util::truncate_with_ellipsis(&content, CROSS_CHAT_SNIPPET_CHARS) + } else { + content + }; + let prov = provenance_tag(&sid); + if !appended_cross_header { + // The header explicitly labels these snippets as historical so + // the model down-weights them when answering capability + // questions — see CROSS_CHAT_HEADER doc for the rationale and + // the cross-module wording contract. + if context.len() + CROSS_CHAT_HEADER.len() > budget { + break; + } + context.push_str(CROSS_CHAT_HEADER); + appended_cross_header = true; + } + let line = format!("- [{prov}] {snippet}\n"); + if context.len() + line.len() > budget { + tracing::debug!( + budget, + current_len = context.len(), + skipped_line_len = line.len(), + "[memory_loader] context budget reached while appending cross-chat context" + ); + break; + } + context.push_str(&line); + } + + if context.is_empty() { + return Ok(String::new()); + } + context.push('\n'); + Ok(context) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::openhuman::memory::{Memory, MemoryCategory, MemoryEntry}; + + struct MockMemory { + entries: Vec, + cross_chat: Vec, + } + + impl MockMemory { + fn new(entries: Vec) -> Self { + Self { + entries, + cross_chat: Vec::new(), + } + } + } + + #[async_trait] + impl Memory for MockMemory { + fn name(&self) -> &str { + "mock" + } + + async fn store( + &self, + _namespace: &str, + _key: &str, + _content: &str, + _category: MemoryCategory, + _session_id: Option<&str>, + ) -> anyhow::Result<()> { + Ok(()) + } + + async fn recall( + &self, + _query: &str, + _limit: usize, + opts: crate::openhuman::memory::RecallOpts<'_>, + ) -> anyhow::Result> { + if opts.cross_session { + return Ok(self.cross_chat.clone()); + } + Ok(self.entries.clone()) + } + + async fn get(&self, _namespace: &str, _key: &str) -> anyhow::Result> { + Ok(None) + } + + async fn list( + &self, + _namespace: Option<&str>, + _category: Option<&MemoryCategory>, + _session_id: Option<&str>, + ) -> anyhow::Result> { + Ok(Vec::new()) + } + + async fn forget(&self, _namespace: &str, _key: &str) -> anyhow::Result { + Ok(false) + } + + async fn namespace_summaries( + &self, + ) -> anyhow::Result> { + Ok(Vec::new()) + } + + async fn count(&self) -> anyhow::Result { + Ok(self.entries.len()) + } + + async fn health_check(&self) -> bool { + true + } + } + + fn entry(key: &str, content: &str, score: Option) -> MemoryEntry { + MemoryEntry { + id: format!("id-{key}"), + key: key.to_string(), + content: content.to_string(), + namespace: Some("test".to_string()), + category: MemoryCategory::Conversation, + timestamp: "2026-04-22T00:00:00Z".to_string(), + session_id: None, + score, + taint: Default::default(), + } + } + + #[test] + fn memory_entry_date_label_parses_rfc3339_else_none() { + assert_eq!( + super::memory_entry_date_label("2026-05-25T07:00:00Z").as_deref(), + Some("2026-05-25") + ); + assert_eq!(super::memory_entry_date_label("not-a-date"), None); + assert_eq!(super::memory_entry_date_label(""), None); + } + + #[tokio::test] + async fn loader_stamps_working_memory_with_date() { + // #2944: working-memory facts must carry their last-updated date so + // the model (and downstream sub-agents / the cron briefing, which + // inherit this block) can tell a stale fact from a current one. + let mem = MockMemory::new(vec![MemoryEntry { + id: "id-tz".into(), + key: "working.user.commitment".into(), + content: "Finish the proposal by Wednesday.".into(), + namespace: Some("test".into()), + category: MemoryCategory::Conversation, + timestamp: "2026-05-25T00:00:00Z".into(), + session_id: None, + score: None, + taint: Default::default(), + }]); + + let out = DefaultMemoryLoader::default() + .load_context(&mem, "what's on my plate?") + .await + .expect("loader must succeed"); + + assert!( + out.contains("[User working memory]"), + "expected working-memory block, got:\n{out}" + ); + assert!( + out.contains("(as of 2026-05-25)"), + "working-memory fact must carry its date (#2944), got:\n{out}" + ); + } + + #[tokio::test] + async fn loader_stamps_prior_conversation_with_date() { + // #2944: high-importance prior-chat facts must be dated so a + // months-old statement isn't read as a present-tense commitment. + let mem = MockMemory::new(vec![MemoryEntry { + id: "id-1".into(), + key: "high.preference.aaaaaaaaaaaa".into(), + content: "[high preference] I prefer Postgres for new services.".into(), + namespace: Some(super::CONVERSATION_MEMORY_NAMESPACE.to_string()), + category: MemoryCategory::Conversation, + timestamp: "2026-04-22T00:00:00Z".into(), + session_id: Some("thr_old".into()), + score: Some(0.9), + taint: Default::default(), + }]); + + let out = DefaultMemoryLoader::default() + .load_context(&mem, "what should I default to for storage?") + .await + .expect("loader must succeed"); + + assert!( + out.contains("[Prior conversations]"), + "expected prior conversations block, got:\n{out}" + ); + assert!( + out.contains("(noted 2026-04-22)"), + "prior-conversation fact must carry its date (#2944), got:\n{out}" + ); + } + + #[tokio::test] + async fn loader_surfaces_prior_conversation_high_importance_only() { + // Prior chat extracted two memories: one high-importance preference + // and one medium-importance unresolved task. Only the high one + // should make it into the loader's prompt block (#1399). + let mem = MockMemory::new(vec![ + MemoryEntry { + id: "id-1".into(), + key: "high.preference.aaaaaaaaaaaa".into(), + content: "[high preference] I prefer Postgres for new services.\n[provenance] {\"thread_id\":\"thr_old\"}".into(), + namespace: Some(super::CONVERSATION_MEMORY_NAMESPACE.to_string()), + category: MemoryCategory::Conversation, + timestamp: "2026-04-22T00:00:00Z".into(), + session_id: Some("thr_old".into()), + score: Some(0.9), + taint: Default::default(), + }, + MemoryEntry { + id: "id-2".into(), + key: "med.unresolved_task.bbbbbbbbbbbb".into(), + content: "[med unresolved_task] still need to migrate auth.".into(), + namespace: Some(super::CONVERSATION_MEMORY_NAMESPACE.to_string()), + category: MemoryCategory::Conversation, + timestamp: "2026-04-22T00:00:00Z".into(), + session_id: None, + score: Some(0.9), + taint: Default::default(), + }, + ]); + + let loader = DefaultMemoryLoader::default(); + let out = loader + .load_context(&mem, "what should I default to for storage?") + .await + .expect("loader must succeed"); + + assert!( + out.contains("[Prior conversations]"), + "expected prior conversations block, got:\n{out}" + ); + assert!(out.contains("Postgres")); + assert!( + !out.contains("migrate auth"), + "med-importance entries must not auto-surface, got:\n{out}" + ); + assert!( + !out.contains("[provenance]"), + "provenance is not rendered into the prompt block, got:\n{out}" + ); + } + + #[tokio::test] + async fn collect_recall_citations_filters_and_truncates_entries() { + let mem = MockMemory::new(vec![ + entry("keep", "useful context", Some(0.9)), + entry("drop", "too weak", Some(0.1)), + entry("long", &"x".repeat(600), Some(0.8)), + ]); + + let citations = collect_recall_citations(&mem, "hello", 5, 0.4) + .await + .expect("citation collection should succeed"); + assert_eq!(citations.len(), 2); + assert_eq!(citations[0].key, "keep"); + assert_eq!(citations[1].key, "long"); + assert!(citations[1].snippet.ends_with("...")); + } + + // ── Cross-chat context (#1505) ─────────────────────────────────────── + + fn cross_chat_entry( + cross_id: &str, + session_id: &str, + content: &str, + score: Option, + ) -> MemoryEntry { + MemoryEntry { + id: format!("episodic-cross:{cross_id}"), + key: format!("{session_id}:user"), + content: content.into(), + namespace: None, + category: MemoryCategory::Conversation, + timestamp: "2026-05-15T00:00:00Z".into(), + session_id: Some(session_id.into()), + score, + taint: Default::default(), + } + } + + #[tokio::test] + async fn loader_surfaces_cross_chat_block_with_provenance_tag() { + let mut mem = MockMemory::new(Vec::new()); + mem.cross_chat = vec![cross_chat_entry( + "1", + "thread-source", + "I prefer Postgres for new services", + Some(0.9), + )]; + + let loader = DefaultMemoryLoader::default(); + let out = loader + .load_context(&mem, "what database should I use?") + .await + .expect("loader must succeed"); + assert!( + out.contains(CROSS_CHAT_HEADER.trim_end()), + "expected cross-chat header, got:\n{out}" + ); + assert!( + out.contains("Postgres"), + "expected the cross-chat fact in the loader output, got:\n{out}" + ); + assert!( + out.contains("[chat:"), + "expected provenance tag, got:\n{out}" + ); + assert!( + !out.contains("thread-source"), + "raw session id MUST NOT leak into the prompt — render only the hashed tag, got:\n{out}" + ); + } + + #[tokio::test] + async fn loader_caps_cross_chat_block_at_limit() { + let mut mem = MockMemory::new(Vec::new()); + mem.cross_chat = (0..10) + .map(|i| { + cross_chat_entry( + &format!("{i}"), + &format!("thread-{i}"), + &format!("Cross-chat fact #{i}"), + Some(0.9), + ) + }) + .collect(); + + let loader = DefaultMemoryLoader::default(); + let out = loader + .load_context(&mem, "Cross-chat fact") + .await + .expect("loader must succeed"); + let cross_lines = out.lines().filter(|l| l.starts_with("- [chat:")).count(); + assert!( + cross_lines <= CROSS_CHAT_LIMIT, + "loader cross-chat block must be capped at {CROSS_CHAT_LIMIT}, saw {cross_lines}" + ); + } + + #[tokio::test] + async fn loader_drops_cross_chat_below_relevance_threshold() { + let mut mem = MockMemory::new(Vec::new()); + mem.cross_chat = vec![ + cross_chat_entry("1", "thread-a", "low score chat fact", Some(0.05)), + cross_chat_entry("2", "thread-b", "high score chat fact", Some(0.9)), + ]; + + let loader = DefaultMemoryLoader::default(); + let out = loader + .load_context(&mem, "fact") + .await + .expect("loader must succeed"); + assert!( + out.contains("high score chat fact"), + "high-relevance cross-chat must surface, got:\n{out}" + ); + assert!( + !out.contains("low score chat fact"), + "low-relevance cross-chat must be filtered, got:\n{out}" + ); + } + + #[tokio::test] + async fn loader_returns_empty_when_no_cross_chat_or_other_blocks_match() { + let mem = MockMemory::new(Vec::new()); + let loader = DefaultMemoryLoader::default(); + let out = loader + .load_context(&mem, "anything") + .await + .expect("loader must succeed"); + assert!( + !out.contains(CROSS_CHAT_HEADER.trim_end()), + "no cross-chat hits must produce no header, got:\n{out}" + ); + } + + /// Exercises the **primary** cross-chat path (JSONL scan via + /// `ConversationStore`, not the `Memory::recall` fallback). Writes + /// two threads through `ConversationStore`, wires `workspace_dir` + /// into the loader, and asserts the prompt picks up the hit from + /// the inactive thread with a redacted provenance tag. + /// + /// Production-critical because the fallback `MockMemory` path is + /// what the other loader tests cover — this is the one users + /// actually run. + #[tokio::test] + async fn loader_surfaces_jsonl_primary_path_with_workspace_dir() { + use crate::openhuman::memory_conversations::{ + ConversationMessage, ConversationStore, CreateConversationThread, + }; + + let temp = tempfile::TempDir::new().expect("tempdir"); + let store = ConversationStore::new(temp.path().to_path_buf()); + + // Chat A — durable fact lives here. + store + .ensure_thread(CreateConversationThread { + parent_thread_id: None, + id: "thread-a".to_string(), + title: "Chat A".to_string(), + created_at: "2026-04-10T12:00:00Z".to_string(), + labels: None, + personality_id: None, + }) + .expect("ensure thread-a"); + store + .append_message( + "thread-a", + ConversationMessage { + id: "m-a-1".to_string(), + content: "Remember: my project Phoenix uses Go and PostgreSQL.".to_string(), + message_type: "text".to_string(), + extra_metadata: serde_json::json!({}), + sender: "user".to_string(), + created_at: "2026-04-10T12:01:00Z".to_string(), + }, + ) + .expect("append a"); + + // Chat B — active chat (excluded by current_thread_id wiring is + // not exercised here; we just verify the JSONL path surfaces + // hits from other threads). + store + .ensure_thread(CreateConversationThread { + parent_thread_id: None, + id: "thread-b".to_string(), + title: "Chat B".to_string(), + created_at: "2026-04-10T13:00:00Z".to_string(), + labels: None, + personality_id: None, + }) + .expect("ensure thread-b"); + + // MockMemory's cross_chat list is empty — if the loader fell + // back to the Memory::recall path we'd render nothing. Forcing + // a JSONL primary hit proves the workspace_dir branch ran. + let mem = MockMemory::new(Vec::new()); + let loader = DefaultMemoryLoader::new(5, 0.4).with_workspace_dir(temp.path().to_path_buf()); + + let out = loader + .load_context(&mem, "What database does my project Phoenix use") + .await + .expect("loader must succeed"); + + assert!( + out.contains(CROSS_CHAT_HEADER.trim_end()), + "JSONL primary path must emit the cross-chat header, got:\n{out}" + ); + assert!( + out.contains("PostgreSQL"), + "cross-chat block must carry the matched snippet, got:\n{out}" + ); + assert!( + out.contains("chat:"), + "cross-chat block must render a `chat:` provenance tag, got:\n{out}" + ); + assert!( + !out.contains("thread-a"), + "raw thread_id must not leak into the prompt, got:\n{out}" + ); + } +} diff --git a/src/openhuman/agent_memory/mod.rs b/src/openhuman/agent_memory/mod.rs new file mode 100644 index 000000000..8e092943a --- /dev/null +++ b/src/openhuman/agent_memory/mod.rs @@ -0,0 +1,13 @@ +//! Memory agent domain — owns the memory retrieval agent, its prompt, benchmarking, +//! and performance instrumentation for memory tree walking and chunk retrieval. +//! +//! The memory agent is a specialist that navigates the user's memory tree, +//! combining vector search, keyword matching, entity lookup, and hierarchical +//! tree browsing to answer queries. This domain centralizes the agent definition, +//! prompt construction, and retrieval performance tracking. + +pub mod agent; +pub mod memory_loader; +pub mod ops; +pub mod tools; +pub mod types; diff --git a/src/openhuman/agent_memory/ops.rs b/src/openhuman/agent_memory/ops.rs new file mode 100644 index 000000000..939cc5194 --- /dev/null +++ b/src/openhuman/agent_memory/ops.rs @@ -0,0 +1,173 @@ +//! Memory agent operations — benchmarking harness for memory tree walking +//! and retrieval performance measurement. + +use crate::openhuman::agent_memory::types::{BenchmarkSummary, RetrievalStep, WalkBenchmark}; +use crate::openhuman::config::Config; +use crate::openhuman::inference::provider::traits::Provider; +use crate::openhuman::memory::query::smart_walk::{ + run_smart_walk, SmartWalkOptions, SmartWalkStopReason, +}; +use std::path::PathBuf; +use std::time::Instant; + +/// Run a single benchmarked smart walk against the memory tree. +pub async fn bench_walk( + config: &Config, + provider: &dyn Provider, + query: &str, + namespace: &str, + content_root: Option, + max_turns: usize, + model: Option, +) -> anyhow::Result { + let effective_root = content_root + .clone() + .unwrap_or_else(|| config.memory_tree_content_root()); + + log::info!( + "[agent_memory::bench] query_len={} namespace={} content_root={} max_turns={}", + query.len(), + namespace, + effective_root.display(), + max_turns + ); + + let opts = SmartWalkOptions { + max_turns, + namespace: namespace.to_string(), + model, + content_root: content_root.clone(), + }; + + let start = Instant::now(); + let outcome = run_smart_walk(config, provider, query, opts).await?; + let total_elapsed = start.elapsed(); + + let steps: Vec = outcome + .trace + .iter() + .map(|step| RetrievalStep { + turn: step.turn, + action: step.action.clone(), + args_summary: step.args_summary.clone(), + result_preview: step.result_preview.clone(), + elapsed: std::time::Duration::ZERO, // per-step timing not available from smart_walk yet + chunks_returned: 0, + bytes_scanned: 0, + }) + .collect(); + + let total_chunks = outcome.evidence.len(); + + let stop_reason = match &outcome.stopped_reason { + SmartWalkStopReason::Answered => "answered".to_string(), + SmartWalkStopReason::MaxTurnsReached => "max_turns_reached".to_string(), + SmartWalkStopReason::LlmGaveUp => "llm_gave_up".to_string(), + SmartWalkStopReason::Error(e) => format!("error: {e}"), + }; + + let benchmark = WalkBenchmark { + query: query.to_string(), + namespace: namespace.to_string(), + content_root: effective_root.display().to_string(), + total_elapsed, + steps, + total_turns: outcome.turns_used, + total_chunks_retrieved: total_chunks, + total_bytes_scanned: outcome + .evidence + .iter() + .map(|e| e.snippet.len() as u64) + .sum(), + answer: outcome.answer, + stop_reason, + }; + + log::info!( + "[agent_memory::bench] completed query_len={} elapsed={:?} turns={} chunks={} stop={}", + query.len(), + total_elapsed, + benchmark.total_turns, + benchmark.total_chunks_retrieved, + benchmark.stop_reason + ); + + Ok(benchmark) +} + +/// Run a batch of queries and produce a summary. +pub async fn bench_batch( + config: &Config, + provider: &dyn Provider, + queries: &[&str], + namespace: &str, + content_root: Option, + max_turns: usize, + model: Option, +) -> anyhow::Result<(Vec, BenchmarkSummary)> { + let mut results = Vec::with_capacity(queries.len()); + + for query in queries { + match bench_walk( + config, + provider, + query, + namespace, + content_root.clone(), + max_turns, + model.clone(), + ) + .await + { + Ok(bench) => results.push(bench), + Err(e) => { + log::warn!( + "[agent_memory::bench_batch] query={:?} failed: {e:#}", + query + ); + } + } + } + + if results.is_empty() && !queries.is_empty() { + anyhow::bail!( + "[agent_memory::bench_batch] all {} queries failed", + queries.len() + ); + } + + let summary = BenchmarkSummary::from_benchmarks(&results); + Ok((results, summary)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn empty_summary_is_zeroed() { + let summary = BenchmarkSummary::from_benchmarks(&[]); + assert_eq!(summary.runs, 0); + assert_eq!(summary.avg_elapsed_ms, 0.0); + } + + #[test] + fn summary_from_single_run() { + let bench = WalkBenchmark { + query: "test".into(), + namespace: "default".into(), + content_root: "/tmp".into(), + total_elapsed: std::time::Duration::from_millis(500), + steps: vec![], + total_turns: 3, + total_chunks_retrieved: 5, + total_bytes_scanned: 1024, + answer: "test answer".into(), + stop_reason: "answered".into(), + }; + let summary = BenchmarkSummary::from_benchmarks(&[bench]); + assert_eq!(summary.runs, 1); + assert!((summary.avg_elapsed_ms - 500.0).abs() < 1.0); + assert!((summary.avg_turns - 3.0).abs() < 0.01); + } +} diff --git a/src/openhuman/agent_memory/tools.rs b/src/openhuman/agent_memory/tools.rs new file mode 100644 index 000000000..2b89357a4 --- /dev/null +++ b/src/openhuman/agent_memory/tools.rs @@ -0,0 +1,223 @@ +//! Tool: `call_memory_agent` — invoke the memory retrieval agent to walk +//! the memory tree and return context for a query. +//! +//! Unlike the lower-level `memory_tree`, `memory_smart_walk`, and +//! `memory_tree_walk` tools which are individual retrieval primitives, +//! this tool spawns the full `agent_memory` sub-agent which autonomously +//! decides which retrieval strategies to combine, navigates the tree +//! across multiple turns, and returns a synthesised, cited answer. +//! +//! Supports both sync (blocking) and async (fire-and-forget) modes. + +use crate::openhuman::agent::harness::definition::AgentDefinitionRegistry; +use crate::openhuman::agent::harness::fork_context::current_parent; +use crate::openhuman::agent::harness::subagent_runner::{ + run_subagent, SubagentRunOptions, SubagentRunStatus, +}; +use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolCategory, ToolResult, ToolScope}; +use async_trait::async_trait; +use serde_json::json; + +const AGENT_ID: &str = "agent_memory"; + +pub struct CallMemoryAgentTool; + +impl CallMemoryAgentTool { + pub fn new() -> Self { + Self + } +} + +impl Default for CallMemoryAgentTool { + fn default() -> Self { + Self::new() + } +} + +#[async_trait] +impl Tool for CallMemoryAgentTool { + fn name(&self) -> &str { + "call_memory_agent" + } + + fn description(&self) -> &str { + "Invoke the memory retrieval agent to walk the memory tree and \ + return relevant context for a query. The agent autonomously \ + combines vector search, keyword matching, entity lookup, and \ + tree browsing — then returns a cited answer. Use this when you \ + need a comprehensive memory search rather than a single-strategy \ + lookup." + } + + fn parameters_schema(&self) -> serde_json::Value { + json!({ + "type": "object", + "required": ["query"], + "properties": { + "query": { + "type": "string", + "description": "Natural-language query to search the user's memory for." + }, + "context": { + "type": "string", + "description": "Optional context to help the memory agent understand what you're looking for and why." + }, + "max_turns": { + "type": "integer", + "description": "Max retrieval turns the memory agent can take. Default: 15, hard cap: 20.", + "minimum": 1, + "maximum": 20 + }, + "async": { + "type": "boolean", + "description": "If true, fire-and-forget — the agent runs in the background and results are not returned inline. Default: false (synchronous)." + } + } + }) + } + + fn category(&self) -> ToolCategory { + ToolCategory::System + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::ReadOnly + } + + fn scope(&self) -> ToolScope { + ToolScope::AgentOnly + } + + fn is_concurrency_safe(&self, _args: &serde_json::Value) -> bool { + true + } + + async fn execute(&self, args: serde_json::Value) -> anyhow::Result { + let query = args + .get("query") + .and_then(|v| v.as_str()) + .ok_or_else(|| anyhow::anyhow!("call_memory_agent: `query` is required"))?; + + let context = args.get("context").and_then(|v| v.as_str()); + + let max_turns = args + .get("max_turns") + .and_then(|v| v.as_u64()) + .map(|v| v.max(1).min(20) as usize); + + let is_async = args.get("async").and_then(|v| v.as_bool()).unwrap_or(false); + + let parent = current_parent(); + if parent.is_none() { + return Ok(ToolResult::error( + "call_memory_agent: no parent agent context — this tool must be \ + called from within an agent turn." + .to_string(), + )); + } + + let registry = AgentDefinitionRegistry::global() + .ok_or_else(|| anyhow::anyhow!("call_memory_agent: agent registry not initialised"))?; + + let definition = registry + .list() + .iter() + .find(|d| d.id == AGENT_ID) + .cloned() + .ok_or_else(|| { + anyhow::anyhow!( + "call_memory_agent: agent definition '{AGENT_ID}' not found in registry" + ) + })?; + + let mut prompt = format!( + "Search the user's memory tree and return relevant context for this query:\n\n{query}" + ); + if let Some(turns) = max_turns { + prompt.push_str(&format!( + "\n\nConstraint: use at most {turns} retrieval turns." + )); + } + if let Some(ctx) = context { + prompt.push_str(&format!("\n\nAdditional context:\n{ctx}")); + } + + let task_id = format!("mem-{}", uuid::Uuid::new_v4()); + + log::debug!( + "[call_memory_agent] query_len={} async={} task_id={}", + query.len(), + is_async, + task_id + ); + + let options = SubagentRunOptions { + task_id: Some(task_id.clone()), + ..Default::default() + }; + + if is_async { + let def = definition.clone(); + let prompt_clone = prompt.clone(); + let tid = task_id.clone(); + tokio::spawn(async move { + match run_subagent(&def, &prompt_clone, options).await { + Ok(outcome) => { + log::info!( + "[call_memory_agent] async task_id={} completed iterations={} elapsed={:?}", + tid, + outcome.iterations, + outcome.elapsed + ); + } + Err(e) => { + log::warn!("[call_memory_agent] async task_id={} failed: {e:#}", tid); + } + } + }); + + return Ok(ToolResult::success(format!( + "Memory agent dispatched asynchronously (task_id: {task_id}). \ + Results will be available when the agent completes." + ))); + } + + // Synchronous path — block until the memory agent finishes. + let started = std::time::Instant::now(); + match run_subagent(&definition, &prompt, options).await { + Ok(outcome) => { + let elapsed = started.elapsed(); + log::info!( + "[call_memory_agent] task_id={} completed iterations={} elapsed={:?} status={:?}", + task_id, + outcome.iterations, + elapsed, + outcome.status + ); + + let mut result = outcome.output; + + match outcome.status { + SubagentRunStatus::Completed => {} + SubagentRunStatus::AwaitingUser { question, .. } => { + result.push_str(&format!( + "\n\n⚠️ The memory agent needs clarification: {question}" + )); + } + } + + result.push_str(&format!( + "\n\n---\n_Memory agent: {} iterations, {:.1}s_", + outcome.iterations, + elapsed.as_secs_f64() + )); + + Ok(ToolResult::success(result)) + } + Err(e) => { + log::warn!("[call_memory_agent] task_id={} failed: {e:#}", task_id); + Ok(ToolResult::error(format!("Memory agent failed: {e}"))) + } + } + } +} diff --git a/src/openhuman/agent_memory/types.rs b/src/openhuman/agent_memory/types.rs new file mode 100644 index 000000000..54feb0fa2 --- /dev/null +++ b/src/openhuman/agent_memory/types.rs @@ -0,0 +1,85 @@ +//! Types for memory agent retrieval performance tracking. + +use serde::{Deserialize, Serialize}; +use std::time::Duration; + +/// A single retrieval operation performed during a memory walk. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct RetrievalStep { + pub turn: usize, + pub action: String, + pub args_summary: String, + pub result_preview: String, + pub elapsed: Duration, + pub chunks_returned: usize, + pub bytes_scanned: u64, +} + +/// Outcome of a benchmarked memory walk. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct WalkBenchmark { + pub query: String, + pub namespace: String, + pub content_root: String, + pub total_elapsed: Duration, + pub steps: Vec, + pub total_turns: usize, + pub total_chunks_retrieved: usize, + pub total_bytes_scanned: u64, + pub answer: String, + pub stop_reason: String, +} + +/// Summary statistics for a batch of benchmark runs. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct BenchmarkSummary { + pub runs: usize, + pub avg_elapsed_ms: f64, + pub p50_elapsed_ms: f64, + pub p95_elapsed_ms: f64, + pub avg_turns: f64, + pub avg_chunks: f64, + pub total_bytes_scanned: u64, +} + +impl BenchmarkSummary { + pub fn from_benchmarks(benchmarks: &[WalkBenchmark]) -> Self { + if benchmarks.is_empty() { + return Self { + runs: 0, + avg_elapsed_ms: 0.0, + p50_elapsed_ms: 0.0, + p95_elapsed_ms: 0.0, + avg_turns: 0.0, + avg_chunks: 0.0, + total_bytes_scanned: 0, + }; + } + + let n = benchmarks.len() as f64; + let mut elapsed_ms: Vec = benchmarks + .iter() + .map(|b| b.total_elapsed.as_secs_f64() * 1000.0) + .collect(); + elapsed_ms.sort_by(|a, b| a.partial_cmp(b).unwrap()); + + let avg_elapsed_ms = elapsed_ms.iter().sum::() / n; + let p50_idx = + ((benchmarks.len() as f64 * 0.5) as usize).min(benchmarks.len().saturating_sub(1)); + let p95_idx = ((benchmarks.len() as f64 * 0.95) as usize).min(benchmarks.len() - 1); + + Self { + runs: benchmarks.len(), + avg_elapsed_ms, + p50_elapsed_ms: elapsed_ms[p50_idx], + p95_elapsed_ms: elapsed_ms[p95_idx], + avg_turns: benchmarks.iter().map(|b| b.total_turns as f64).sum::() / n, + avg_chunks: benchmarks + .iter() + .map(|b| b.total_chunks_retrieved as f64) + .sum::() + / n, + total_bytes_scanned: benchmarks.iter().map(|b| b.total_bytes_scanned).sum(), + } + } +} diff --git a/src/openhuman/agent_registry/agents/crypto_agent/agent.toml b/src/openhuman/agent_registry/agents/crypto_agent/agent.toml index ab7f5f4bf..d5a4a7056 100644 --- a/src/openhuman/agent_registry/agents/crypto_agent/agent.toml +++ b/src/openhuman/agent_registry/agents/crypto_agent/agent.toml @@ -63,10 +63,11 @@ named = [ "stock_quote", "stock_exchange_rate", "stock_crypto_series", - # Memory recall lets the agent ground execution in the user's - # previously-stated preferences (default chain, slippage tolerance, - # named addresses) instead of re-asking every time. - "memory_recall", + # Memory retrieval — delegates to the memory agent for grounding + # execution in the user's previously-stated preferences (default + # chain, slippage tolerance, named addresses) instead of re-asking + # every time. + "call_memory_agent", # Confirmation gate — the agent MUST call this before any # wallet_execute_prepared / write-side exchange order. The runtime # routes the prompt to the user and blocks until they reply. diff --git a/src/openhuman/agent_registry/agents/help/agent.toml b/src/openhuman/agent_registry/agents/help/agent.toml index 977240374..a58508a99 100644 --- a/src/openhuman/agent_registry/agents/help/agent.toml +++ b/src/openhuman/agent_registry/agents/help/agent.toml @@ -21,4 +21,4 @@ omit_memory_md = false hint = "agentic" [tools] -named = ["gitbooks_search", "gitbooks_get_page", "memory_recall"] +named = ["gitbooks_search", "gitbooks_get_page", "call_memory_agent"] diff --git a/src/openhuman/agent_registry/agents/loader.rs b/src/openhuman/agent_registry/agents/loader.rs index 67fff2861..18f309a01 100644 --- a/src/openhuman/agent_registry/agents/loader.rs +++ b/src/openhuman/agent_registry/agents/loader.rs @@ -188,6 +188,11 @@ pub const BUILTINS: &[BuiltinAgent] = &[ toml: include_str!("mcp_setup/agent.toml"), prompt_fn: super::mcp_setup::prompt::build, }, + BuiltinAgent { + id: "agent_memory", + toml: include_str!("../../agent_memory/agent/agent.toml"), + prompt_fn: crate::openhuman::agent_memory::agent::prompt::build, + }, ]; /// Parse every entry in [`BUILTINS`] into an [`AgentDefinition`]. @@ -327,8 +332,8 @@ mod tests { match &def.tools { ToolScope::Named(tools) => { assert!( - tools.iter().any(|t| t == "memory_recall"), - "trigger_reactor needs memory_recall" + tools.iter().any(|t| t == "call_memory_agent"), + "trigger_reactor needs call_memory_agent" ); assert!( tools.iter().any(|t| t == "memory_store"), @@ -492,10 +497,9 @@ mod tests { !tools.iter().any(|t| t == "spawn_subagent"), "spawn_subagent must not appear — removed in #1141" ); - // consolidated memory_tree* → single memory_tree with mode dispatch assert!( - tools.iter().any(|t| t == "memory_tree"), - "orchestrator must have memory_tree" + tools.iter().any(|t| t == "call_memory_agent"), + "orchestrator must have call_memory_agent" ); assert!(!tools.iter().any(|t| t == "shell")); assert!(!tools.iter().any(|t| t == "file_write")); @@ -622,7 +626,7 @@ mod tests { match &presentation.tools { ToolScope::Named(names) => { assert!(names.iter().any(|name| name == "generate_presentation")); - assert!(names.iter().any(|name| name == "memory_tree")); + assert!(names.iter().any(|name| name == "call_memory_agent")); assert!(names.iter().any(|name| name == "web_search_tool")); } other => panic!("presentation_agent must use Named tool scope, got {other:?}"), @@ -679,8 +683,8 @@ mod tests { "help needs gitbooks_get_page" ); assert!( - tools.iter().any(|t| t == "memory_recall"), - "help needs memory_recall for personalisation" + tools.iter().any(|t| t == "call_memory_agent"), + "help needs call_memory_agent for personalisation" ); // Help is docs-only — no write/exec tools. assert!(!tools.iter().any(|t| t == "shell")); @@ -808,7 +812,7 @@ mod tests { ); // Market grounding + context helpers. Pin the full set so a // TOML edit that silently drops `stock_quote`, - // `stock_exchange_rate`, `memory_recall`, or `current_time` + // `stock_exchange_rate`, `call_memory_agent`, or `current_time` // gets caught here — the agent's quote-before-execute // discipline and "ground in user preferences before re-asking" // behaviour both depend on these being present. @@ -816,7 +820,7 @@ mod tests { "stock_quote", "stock_exchange_rate", "stock_crypto_series", - "memory_recall", + "call_memory_agent", "current_time", ] { assert!( @@ -909,10 +913,10 @@ mod tests { "markets_agent needs ask_user_clarification to gate write ops" ); // Context helpers. Pin the full set so a TOML edit that - // silently drops `memory_recall` or `current_time` gets + // silently drops `call_memory_agent` or `current_time` gets // caught here — the agent's "ground in user preferences" // and "as of " framing depend on these. - for required in ["memory_recall", "current_time"] { + for required in ["call_memory_agent", "current_time"] { assert!( tools.iter().any(|t| t == required), "markets_agent needs supporting tool `{required}`" diff --git a/src/openhuman/agent_registry/agents/markets_agent/agent.toml b/src/openhuman/agent_registry/agents/markets_agent/agent.toml index bc380221a..46d5fa819 100644 --- a/src/openhuman/agent_registry/agents/markets_agent/agent.toml +++ b/src/openhuman/agent_registry/agents/markets_agent/agent.toml @@ -32,10 +32,10 @@ named = [ # Prediction-market venues. "polymarket", "kalshi", - # Memory recall lets the agent ground execution in the user's - # previously-stated preferences (default venue, account labels) - # instead of re-asking every time. - "memory_recall", + # Memory retrieval — delegates to the memory agent for grounding + # execution in the user's previously-stated preferences (default + # venue, account labels) instead of re-asking every time. + "call_memory_agent", # Confirmation gate — surfaced to the user when a venue tool returns # the approval-required error. The runtime routes the prompt to the # user and blocks until they reply. diff --git a/src/openhuman/agent_registry/agents/orchestrator/agent.toml b/src/openhuman/agent_registry/agents/orchestrator/agent.toml index 2a61a2601..ce063e732 100644 --- a/src/openhuman/agent_registry/agents/orchestrator/agent.toml +++ b/src/openhuman/agent_registry/agents/orchestrator/agent.toml @@ -141,8 +141,7 @@ hint = "chat" # the orchestrator never calls composio_authorize / composio_list_tools # / composio_execute directly. named = [ - "query_memory", - "memory_tree", + "call_memory_agent", "read_workspace_state", "ask_user_clarification", "spawn_worker_thread", diff --git a/src/openhuman/agent_registry/agents/planner/agent.toml b/src/openhuman/agent_registry/agents/planner/agent.toml index 41946f08f..202401772 100644 --- a/src/openhuman/agent_registry/agents/planner/agent.toml +++ b/src/openhuman/agent_registry/agents/planner/agent.toml @@ -47,7 +47,7 @@ named = [ "todowrite", "plan_exit", "web_fetch", - "memory_recall", + "call_memory_agent", "web_search_tool", # Grounded research + market-data lookups so plans can be anchored in # real numbers (stock/crypto prices) and current web context, not just diff --git a/src/openhuman/agent_registry/agents/presentation_agent/agent.toml b/src/openhuman/agent_registry/agents/presentation_agent/agent.toml index a37061a27..07571b95b 100644 --- a/src/openhuman/agent_registry/agents/presentation_agent/agent.toml +++ b/src/openhuman/agent_registry/agents/presentation_agent/agent.toml @@ -18,8 +18,7 @@ hint = "agentic" [tools] named = [ "generate_presentation", - "memory_tree", - "query_memory", + "call_memory_agent", "web_search_tool", "web_fetch", "http_request", diff --git a/src/openhuman/agent_registry/agents/profile_memory_agent/agent.toml b/src/openhuman/agent_registry/agents/profile_memory_agent/agent.toml index 1f0ece4e7..8898e8c46 100644 --- a/src/openhuman/agent_registry/agents/profile_memory_agent/agent.toml +++ b/src/openhuman/agent_registry/agents/profile_memory_agent/agent.toml @@ -18,7 +18,7 @@ hint = "agentic" [tools] named = [ - "memory_recall", + "call_memory_agent", "memory_store", "memory_forget", "memory_doctor", @@ -26,8 +26,6 @@ named = [ "memory_tools_put", "remember_preference", "save_preference", - "query_memory", - "memory_tree", "workspace_read_persona", "workspace_update_persona", "workspace_reset_persona", diff --git a/src/openhuman/agent_registry/agents/researcher/agent.toml b/src/openhuman/agent_registry/agents/researcher/agent.toml index 9023a6c07..d34b4165a 100644 --- a/src/openhuman/agent_registry/agents/researcher/agent.toml +++ b/src/openhuman/agent_registry/agents/researcher/agent.toml @@ -30,7 +30,7 @@ named = [ "grep", "glob", "list", - "memory_recall", + "call_memory_agent", # Parallel — full surface (search/extract are also delegated by web_search_tool; # chat/research/enrich/dataset unlock grounded answers and deep multi-step research). "parallel_search", diff --git a/src/openhuman/agent_registry/agents/trigger_reactor/agent.toml b/src/openhuman/agent_registry/agents/trigger_reactor/agent.toml index 4bff8d016..191457810 100644 --- a/src/openhuman/agent_registry/agents/trigger_reactor/agent.toml +++ b/src/openhuman/agent_registry/agents/trigger_reactor/agent.toml @@ -31,12 +31,12 @@ hint = "agentic" [tools] # Small, deliberately narrow tool surface: -# - memory_recall / memory_store: look up and persist context +# - call_memory_agent / memory_store: look up and persist context # - read_workspace_state: understand what the user is working on # - spawn_subagent: escalate to a real specialist if the reaction # turns out to be bigger than the triage agent expected named = [ - "memory_recall", + "call_memory_agent", "memory_store", "read_workspace_state", "spawn_subagent", diff --git a/src/openhuman/mcp_server/resources.rs b/src/openhuman/mcp_server/resources.rs index f9a2272d6..a2d871c58 100644 --- a/src/openhuman/mcp_server/resources.rs +++ b/src/openhuman/mcp_server/resources.rs @@ -205,6 +205,12 @@ const RESOURCE_CATALOG: &[PromptResource] = &[ description: "Specialist worker for screen context and desktop state inspection.", content: include_str!("../agent_registry/agents/screen_awareness_agent/prompt.md"), }, + PromptResource { + uri: "openhuman://prompts/agents/agent_memory", + name: "agent_memory", + description: "Dedicated memory retrieval subagent using smart-walk strategies.", + content: include_str!("../agent_memory/agent/prompt.md"), + }, ]; /// Returns the `resources/list` result payload listing every catalog entry. diff --git a/src/openhuman/memory/query/smart_walk.rs b/src/openhuman/memory/query/smart_walk.rs index bd68f592d..e05bcaf43 100644 --- a/src/openhuman/memory/query/smart_walk.rs +++ b/src/openhuman/memory/query/smart_walk.rs @@ -1135,8 +1135,17 @@ Use a multi-strategy approach inspired by graph-based retrieval: - Prefer keyword_search for specific names, IDs, or exact phrases. - Use entity_search when the query mentions people, projects, or organizations. - Always collect_evidence before answering, so your answer has citations. -- Use XML tool_call tags for actions. -- You can call multiple tools in one turn by including multiple blocks."# +- Use tags with JSON content for actions. Format: + {"name":"tool_name","arguments":{"param":"value"}} +- You can call multiple tools in one turn by including multiple blocks. + +## Example turn + +I'll search for recent emails about the project. + +{"name":"list_sources","arguments":{"content_type":"all"}} +{"name":"keyword_search","arguments":{"pattern":"project","content_type":"raw"}} +"# .into() } @@ -1253,18 +1262,9 @@ fn parse_tool_calls(response: &str) -> (String, Vec) { match after_open.find(CLOSE) { None => break, Some(close_idx) => { - let inner = &after_open[..close_idx]; - if let Ok(val) = serde_json::from_str::(inner.trim()) { - if let Some(name) = val.get("name").and_then(|v| v.as_str()) { - let args = val - .get("arguments") - .cloned() - .unwrap_or(serde_json::Value::Object(Default::default())); - calls.push(InnerCall { - name: name.to_string(), - args, - }); - } + let inner = after_open[..close_idx].trim(); + if let Some(call) = parse_single_tool_call(inner) { + calls.push(call); } remaining = &after_open[close_idx + CLOSE.len()..]; } @@ -1277,6 +1277,77 @@ fn parse_tool_calls(response: &str) -> (String, Vec) { (text_before, calls) } +fn parse_single_tool_call(inner: &str) -> Option { + // Primary: JSON format {"name":"...","arguments":{...}} + if let Ok(val) = serde_json::from_str::(inner) { + if let Some(name) = val.get("name").and_then(|v| v.as_str()) { + let args = val + .get("arguments") + .cloned() + .unwrap_or(serde_json::Value::Object(Default::default())); + log::debug!( + "[smart_walk::parse_single_tool_call] json path: tool={} args_keys={}", + name, + args.as_object().map(|m| m.len()).unwrap_or(0) + ); + return Some(InnerCall { + name: name.to_string(), + args, + }); + } + } + // Fallback: XML-style nameJSON + if let (Some(name), args) = ( + extract_xml_tag(inner, "tool_name"), + extract_xml_tag(inner, "parameters"), + ) { + log::debug!( + "[smart_walk::parse_single_tool_call] xml fallback path: tool={} has_params={}", + name.trim(), + args.is_some() + ); + let parsed_args = args + .and_then(|a| serde_json::from_str::(a.trim()).ok()) + .unwrap_or_else(|| { + // Parameters might be XML key-value pairs; parse them heuristically + let mut map = serde_json::Map::new(); + for line in inner.lines() { + let trimmed = line.trim(); + if trimmed.starts_with('<') + && !trimmed.starts_with("') { + let tag = &trimmed[1..tag_end]; + if let Some(close) = trimmed.find(&format!("")) { + let value = &trimmed[tag_end + 1..close]; + map.insert( + tag.to_string(), + serde_json::Value::String(value.to_string()), + ); + } + } + } + } + serde_json::Value::Object(map) + }); + return Some(InnerCall { + name: name.trim().to_string(), + args: parsed_args, + }); + } + None +} + +fn extract_xml_tag<'a>(text: &'a str, tag: &str) -> Option<&'a str> { + let open = format!("<{tag}>"); + let close = format!(""); + let start = text.find(&open)? + open.len(); + let end = text[start..].find(&close)? + start; + Some(&text[start..end]) +} + // ── Fallback synthesis ────────────────────────────────────────────────────── fn synthesize_fallback(trace: &[SmartWalkStep], evidence: &[Evidence]) -> String { @@ -1676,6 +1747,432 @@ mod tests { } } + // ── Parser tests: XML-format tool calls ──────────────────────────── + + #[test] + fn parse_xml_style_tool_call() { + let response = r#"I'll browse the tree. + +browse_tree +{"node_id":"root"} +"#; + + let (text, calls) = parse_tool_calls(response); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].name, "browse_tree"); + assert_eq!(calls[0].args["node_id"], "root"); + assert!(text.contains("browse the tree")); + } + + #[test] + fn parse_xml_style_with_xml_params() { + let response = r#" +keyword_search + +project status +raw + +"#; + + let (_text, calls) = parse_tool_calls(response); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].name, "keyword_search"); + assert_eq!(calls[0].args["pattern"], "project status"); + assert_eq!(calls[0].args["content_type"], "raw"); + } + + #[test] + fn parse_mixed_json_and_xml_tool_calls() { + let response = r#"Searching... +{"name":"list_sources","arguments":{"content_type":"all"}} + +keyword_search +{"pattern":"email","content_type":"raw"} +"#; + + let (_, calls) = parse_tool_calls(response); + assert_eq!(calls.len(), 2); + assert_eq!(calls[0].name, "list_sources"); + assert_eq!(calls[1].name, "keyword_search"); + assert_eq!(calls[1].args["pattern"], "email"); + } + + #[test] + fn parse_xml_no_parameters_tag() { + let response = r#" +list_sources +"#; + + let (_, calls) = parse_tool_calls(response); + // No tag → extract_xml_tag returns None for parameters + // parse_single_tool_call requires tool_name match but parameters is + // Option, so it should still parse with empty args + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].name, "list_sources"); + } + + #[test] + fn extract_xml_tag_basic() { + assert_eq!(extract_xml_tag("hello", "name"), Some("hello")); + assert_eq!(extract_xml_tag("no tags here", "name"), None); + assert_eq!(extract_xml_tag("12", "b"), Some("2")); + } + + #[test] + fn parse_single_tool_call_json() { + let call = + parse_single_tool_call(r#"{"name":"keyword_search","arguments":{"pattern":"test"}}"#); + assert!(call.is_some()); + let call = call.unwrap(); + assert_eq!(call.name, "keyword_search"); + assert_eq!(call.args["pattern"], "test"); + } + + #[test] + fn parse_single_tool_call_xml() { + let call = parse_single_tool_call( + "read_content\n{\"path\":\"raw/email/test.md\"}", + ); + assert!(call.is_some()); + let call = call.unwrap(); + assert_eq!(call.name, "read_content"); + assert_eq!(call.args["path"], "raw/email/test.md"); + } + + #[test] + fn parse_single_tool_call_garbage_returns_none() { + assert!(parse_single_tool_call("just some text").is_none()); + assert!(parse_single_tool_call("").is_none()); + } + + // ── E2E walk tests with rich seeded content ───────────────────────── + + fn seed_synced_memory(content_root: &Path) { + // Raw email content + let email_dir = content_root.join("raw").join("email").join("inbox"); + std::fs::create_dir_all(&email_dir).unwrap(); + std::fs::write( + email_dir.join("001_meeting.md"), + "---\nsource_kind: email\nauthor: alice@example.com\ndate: 2026-06-01\n---\n\ + # Team standup notes\n\n\ + Action items:\n\ + - Deploy the auth service refactor by Friday\n\ + - Review PR #342 for the billing module\n\ + - Schedule security audit with external team\n", + ) + .unwrap(); + std::fs::write( + email_dir.join("002_project.md"), + "---\nsource_kind: email\nauthor: bob@example.com\ndate: 2026-06-02\n---\n\ + # Project Phoenix status update\n\n\ + The migration is 80% complete. Remaining:\n\ + - Database schema changes (blocked on DBA review)\n\ + - API versioning for backward compatibility\n\ + - Load testing the new endpoints\n", + ) + .unwrap(); + std::fs::write( + email_dir.join("003_personal.md"), + "---\nsource_kind: email\nauthor: carol@example.com\ndate: 2026-06-03\n---\n\ + # Lunch plans\n\n\ + Hey, want to grab sushi on Thursday? The new place on 5th street \ + got great reviews.\n", + ) + .unwrap(); + + // Episodic memories + let ep_dir = content_root.join("episodic").join("daily"); + std::fs::create_dir_all(&ep_dir).unwrap(); + std::fs::write( + ep_dir.join("2026-06-01.md"), + "---\nkind: episodic\ndate: 2026-06-01\n---\n\ + Worked on the auth service refactor. Had a productive standup.\n\ + Identified three blockers for Project Phoenix.\n", + ) + .unwrap(); + + // Wiki summaries + let wiki_dir = content_root + .join("wiki") + .join("summaries") + .join("email-inbox"); + std::fs::create_dir_all(wiki_dir.join("L1")).unwrap(); + std::fs::write( + wiki_dir.join("L1").join("summary-week-22.md"), + "---\nkind: summary\nlevel: 1\n---\n\ + Week 22 summary: Team focused on Project Phoenix migration \ + and auth service refactor. Key contacts: alice@example.com (standup), \ + bob@example.com (project status), carol@example.com (social).\n", + ) + .unwrap(); + + // Document content + let doc_dir = content_root.join("document").join("notes"); + std::fs::create_dir_all(&doc_dir).unwrap(); + std::fs::write( + doc_dir.join("project-phoenix.md"), + "---\nsource_kind: document\n---\n\ + # Project Phoenix\n\n\ + ## Overview\n\ + Migration from legacy monolith to microservices.\n\n\ + ## Status\n\ + Phase 2 of 3 — data migration and API versioning.\n\n\ + ## Key risks\n\ + - Data integrity during cutover\n\ + - Backward compatibility for mobile clients\n", + ) + .unwrap(); + } + + #[tokio::test] + async fn walk_synced_email_with_keyword_and_evidence() { + let tmp = TempDir::new().unwrap(); + let cfg = test_config(&tmp); + let content_root = cfg.workspace_dir.join("memory_tree").join("content"); + seed_synced_memory(&content_root); + + let provider = StubProvider::new(vec![ + // Turn 1: list sources to discover content + r#"{"name":"list_sources","arguments":{"content_type":"all"}}"#, + // Turn 2: keyword search for "project phoenix" + r#"{"name":"keyword_search","arguments":{"pattern":"project phoenix","content_type":"all"}}"#, + // Turn 3: read the project email and project doc + r#"{"name":"read_content","arguments":{"path":"raw/email/inbox/002_project.md"}} +{"name":"read_content","arguments":{"path":"document/notes/project-phoenix.md"}}"#, + // Turn 4: collect evidence + answer + concat!( + r#"{"name":"collect_evidence","arguments":{"items":["#, + r#"{"source":"raw/email/inbox/002_project.md","snippet":"Migration is 80% complete. Remaining: DB schema, API versioning, load testing.","relevance":"direct project status"},"#, + r#"{"source":"document/notes/project-phoenix.md","snippet":"Phase 2 of 3 — data migration and API versioning.","relevance":"project overview doc"}"#, + r#"]}}"#, + "\n", + r#"{"name":"answer","arguments":{"text":"Project Phoenix is 80% complete (Phase 2 of 3). Remaining work: database schema changes (blocked on DBA review), API versioning for backward compatibility, and load testing new endpoints. Key risks include data integrity during cutover and backward compatibility for mobile clients."}}"#, + ), + ]); + + let opts = SmartWalkOptions { + max_turns: 10, + namespace: "default".into(), + model: Some("test-model".into()), + content_root: Some(content_root), + }; + + let outcome = run_smart_walk( + &cfg, + &provider, + "What is the status of Project Phoenix?", + opts, + ) + .await + .unwrap(); + + assert_eq!(outcome.stopped_reason, SmartWalkStopReason::Answered); + assert!(outcome.answer.contains("80%")); + assert!(outcome.answer.contains("Phoenix")); + assert_eq!(outcome.evidence.len(), 2); + assert!(outcome.evidence[0].source_path.contains("002_project")); + assert!(outcome.evidence[1].source_path.contains("project-phoenix")); + assert_eq!(outcome.turns_used, 4); + } + + #[tokio::test] + async fn walk_with_xml_format_tool_calls() { + let tmp = TempDir::new().unwrap(); + let cfg = test_config(&tmp); + let content_root = cfg.workspace_dir.join("memory_tree").join("content"); + seed_synced_memory(&content_root); + + // Simulate the bug scenario: LLM outputs XML-style tool calls + let provider = StubProvider::new(vec![ + // Turn 1: XML-style list_sources + "\nlist_sources\n{\"content_type\":\"all\"}\n", + // Turn 2: XML-style keyword_search + "\nkeyword_search\n{\"pattern\":\"auth service\",\"content_type\":\"raw\"}\n", + // Turn 3: read + answer (JSON this time — mixed is fine) + r#"{"name":"read_content","arguments":{"path":"raw/email/inbox/001_meeting.md"}}"#, + // Turn 4: answer + r#"{"name":"answer","arguments":{"text":"The auth service refactor needs to be deployed by Friday, as discussed in the team standup."}}"#, + ]); + + let opts = SmartWalkOptions { + max_turns: 10, + namespace: "default".into(), + model: Some("test-model".into()), + content_root: Some(content_root), + }; + + let outcome = run_smart_walk( + &cfg, + &provider, + "What do I need to work on for the auth service?", + opts, + ) + .await + .unwrap(); + + assert_eq!(outcome.stopped_reason, SmartWalkStopReason::Answered); + assert!(outcome.answer.contains("auth service")); + // Verify the XML tool calls were parsed — we should have 4 turns, + // not 1 (which would happen if XML calls were silently dropped) + assert_eq!(outcome.turns_used, 4); + assert!(outcome.trace.len() >= 3); + assert_eq!(outcome.trace[0].action, "list_sources"); + assert_eq!(outcome.trace[1].action, "keyword_search"); + } + + #[tokio::test] + async fn walk_reads_across_content_types() { + let tmp = TempDir::new().unwrap(); + let cfg = test_config(&tmp); + let content_root = cfg.workspace_dir.join("memory_tree").join("content"); + seed_synced_memory(&content_root); + + let provider = StubProvider::new(vec![ + // Turn 1: search for "standup" + r#"{"name":"keyword_search","arguments":{"pattern":"standup","content_type":"all"}}"#, + // Turn 2: read the email and the episodic memory + r#"{"name":"read_content","arguments":{"path":"raw/email/inbox/001_meeting.md"}} +{"name":"read_content","arguments":{"path":"episodic/daily/2026-06-01.md"}}"#, + // Turn 3: also read the wiki summary + r#"{"name":"read_content","arguments":{"path":"wiki/summaries/email-inbox/L1/summary-week-22.md"}}"#, + // Turn 4: collect from all 3 sources + answer + concat!( + r#"{"name":"collect_evidence","arguments":{"items":["#, + r#"{"source":"raw/email/inbox/001_meeting.md","snippet":"Deploy auth service refactor by Friday","relevance":"action item from standup"},"#, + r#"{"source":"episodic/daily/2026-06-01.md","snippet":"Had a productive standup","relevance":"episodic record"},"#, + r#"{"source":"wiki/summaries/email-inbox/L1/summary-week-22.md","snippet":"Team focused on Project Phoenix migration","relevance":"weekly summary"}"#, + r#"]}}"#, + "\n", + r#"{"name":"answer","arguments":{"text":"The standup covered Project Phoenix migration progress, auth service refactor deadlines, and identified three blockers."}}"#, + ), + ]); + + let opts = SmartWalkOptions { + max_turns: 10, + namespace: "default".into(), + model: Some("test-model".into()), + content_root: Some(content_root), + }; + + let outcome = run_smart_walk(&cfg, &provider, "What happened in the standup?", opts) + .await + .unwrap(); + + assert_eq!(outcome.stopped_reason, SmartWalkStopReason::Answered); + assert_eq!(outcome.evidence.len(), 3); + // Evidence from all three content types + let sources: Vec<&str> = outcome + .evidence + .iter() + .map(|e| e.source_path.as_str()) + .collect(); + assert!(sources.iter().any(|s| s.contains("raw/"))); + assert!(sources.iter().any(|s| s.contains("episodic/"))); + assert!(sources.iter().any(|s| s.contains("wiki/"))); + } + + #[tokio::test] + async fn walk_llm_gives_up_uses_fallback() { + let tmp = TempDir::new().unwrap(); + let cfg = test_config(&tmp); + let content_root = cfg.workspace_dir.join("memory_tree").join("content"); + seed_synced_memory(&content_root); + + let provider = StubProvider::new(vec![ + // Turn 1: search finds nothing + r#"{"name":"keyword_search","arguments":{"pattern":"quantum computing","content_type":"all"}}"#, + // Turn 2: LLM gives up with empty response + "", + ]); + + let opts = SmartWalkOptions { + max_turns: 5, + namespace: "default".into(), + model: Some("test-model".into()), + content_root: Some(content_root), + }; + + let outcome = run_smart_walk(&cfg, &provider, "Tell me about quantum computing", opts) + .await + .unwrap(); + + assert_eq!(outcome.stopped_reason, SmartWalkStopReason::LlmGaveUp); + assert!(outcome.evidence.is_empty()); + assert!(outcome.answer.contains("Could not converge")); + } + + #[tokio::test] + async fn walk_direct_answer_without_tools() { + let tmp = TempDir::new().unwrap(); + let cfg = test_config(&tmp); + let content_root = cfg.workspace_dir.join("memory_tree").join("content"); + seed_synced_memory(&content_root); + + let provider = StubProvider::new(vec![ + // LLM directly answers without using any tools + "I don't have enough context to answer that question from your memory.", + ]); + + let opts = SmartWalkOptions { + max_turns: 5, + namespace: "default".into(), + model: Some("test-model".into()), + content_root: Some(content_root), + }; + + let outcome = run_smart_walk(&cfg, &provider, "What's the meaning of life?", opts) + .await + .unwrap(); + + assert_eq!(outcome.stopped_reason, SmartWalkStopReason::Answered); + assert!(outcome.answer.contains("don't have enough context")); + assert_eq!(outcome.turns_used, 1); + assert!(outcome.evidence.is_empty()); + } + + #[tokio::test] + async fn walk_collect_evidence_deduplicates_within_limit() { + let tmp = TempDir::new().unwrap(); + let cfg = test_config(&tmp); + let content_root = cfg.workspace_dir.join("memory_tree").join("content"); + seed_synced_memory(&content_root); + + let provider = StubProvider::new(vec![ + // Turn 1: collect a batch of evidence + concat!( + r#"{"name":"collect_evidence","arguments":{"items":["#, + r#"{"source":"raw/email/inbox/001_meeting.md","snippet":"Deploy auth","relevance":"task"},"#, + r#"{"source":"raw/email/inbox/002_project.md","snippet":"Migration 80%","relevance":"status"}"#, + r#"]}}"#, + ), + // Turn 2: collect more evidence (including a duplicate of the first source) + concat!( + r#"{"name":"collect_evidence","arguments":{"items":["#, + r#"{"source":"document/notes/project-phoenix.md","snippet":"Phase 2 of 3","relevance":"doc"},"#, + r#"{"source":"raw/email/inbox/001_meeting.md","snippet":"Deploy auth (duplicate)","relevance":"task"}"#, + r#"]}}"#, + ), + // Turn 3: answer + r#"{"name":"answer","arguments":{"text":"Summary with evidence items including duplicate source."}}"#, + ]); + + let opts = SmartWalkOptions { + max_turns: 10, + namespace: "default".into(), + model: Some("test-model".into()), + content_root: Some(content_root), + }; + + let outcome = run_smart_walk(&cfg, &provider, "Summarize everything", opts) + .await + .unwrap(); + + assert_eq!(outcome.stopped_reason, SmartWalkStopReason::Answered); + // 2 items from turn 1 + 2 items from turn 2 (one of which duplicates a turn-1 source); + // collect_evidence does not deduplicate, so all 4 items are present. + assert_eq!(outcome.evidence.len(), 4); + } + fn walkdir_first_md(dir: &std::path::Path) -> Option { fn recurse(dir: &std::path::Path) -> Option { for entry in std::fs::read_dir(dir).ok()?.flatten() { diff --git a/src/openhuman/mod.rs b/src/openhuman/mod.rs index b25682e78..2920cf3e5 100644 --- a/src/openhuman/mod.rs +++ b/src/openhuman/mod.rs @@ -19,6 +19,7 @@ pub mod accessibility; pub mod agent; pub mod agent_experience; pub mod agent_meetings; +pub mod agent_memory; pub mod agent_orchestration; pub mod agent_registry; pub mod agent_tool_policy; diff --git a/src/openhuman/tools/mod.rs b/src/openhuman/tools/mod.rs index b4db2dc6b..90650a652 100644 --- a/src/openhuman/tools/mod.rs +++ b/src/openhuman/tools/mod.rs @@ -12,6 +12,7 @@ pub(crate) mod user_filter; pub(crate) mod implementations; pub use crate::openhuman::agent::tools::*; +pub use crate::openhuman::agent_memory::tools::*; pub use crate::openhuman::agent_orchestration::tools::*; pub use crate::openhuman::artifacts::tools::*; pub use crate::openhuman::audio_toolkit::tools::*; diff --git a/src/openhuman/tools/ops.rs b/src/openhuman/tools/ops.rs index 2521ae94d..11d9b9e2a 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -217,6 +217,7 @@ pub fn all_tools_with_runtime( Box::new(MemoryQueryTool), Box::new(MemoryQueryWalkTool), Box::new(SmartMemoryWalkTool), + Box::new(CallMemoryAgentTool::new()), // Explicit user-preference pinning — always registered so the model // can save user-stated preferences regardless of whether the full // inference-based learning subsystem is enabled. The preference diff --git a/tests/agent_retrieval_e2e.rs b/tests/agent_retrieval_e2e.rs index 6291147b2..7c8c96e84 100644 --- a/tests/agent_retrieval_e2e.rs +++ b/tests/agent_retrieval_e2e.rs @@ -14,7 +14,7 @@ //! deserialise → typed retrieval → serialise pipeline that the orchestrator //! relies on, and asserts the data round-trips correctly. //! -//! The orchestrator agent.toml entry registering these tool names is +//! The orchestrator agent.toml entry registering `call_memory_agent` is //! covered by [`orchestrator_lists_memory_tree_tools`] — that catches a //! regression where the tool wrapper exists but the orchestrator can't see //! it. @@ -145,29 +145,28 @@ fn alice_phoenix_thread() -> EmailThread { } } -/// The orchestrator definition must list the consolidated `memory_tree` tool -/// so the bus filter exposes it to the LLM. A wired-up wrapper that's -/// invisible to the orchestrator is dead code. +/// The orchestrator definition must list `call_memory_agent` so memory +/// queries route through the dedicated memory subagent. /// -/// NOTE: #1141 consolidated the 6 individual `memory_tree_*` tools -/// (`memory_tree_search_entities`, `memory_tree_query_topic`, etc.) into a -/// single `memory_tree` tool with a `mode` dispatch parameter. The orchestrator -/// TOML was updated accordingly. +/// History: #1141 consolidated 6 `memory_tree_*` tools into `memory_tree`; +/// the agent_memory domain then unified `memory_tree` + `query_memory` +/// behind `call_memory_agent`. #[test] fn orchestrator_lists_memory_tree_tools() { let toml = include_str!("../src/openhuman/agent_registry/agents/orchestrator/agent.toml"); - // Exact entry match — substring match would also hit comments or prefixed names. - let has_memory_tree_entry = toml + let has_call_memory_agent = toml .lines() .map(str::trim) - .any(|line| line == "\"memory_tree\"" || line == "\"memory_tree\","); + .any(|line| line == "\"call_memory_agent\"" || line == "\"call_memory_agent\","); assert!( - has_memory_tree_entry, - "orchestrator agent.toml must list 'memory_tree' as a named tool entry" + has_call_memory_agent, + "orchestrator agent.toml must list 'call_memory_agent' as a named tool entry" ); - // Verify the old individual tool names are gone — they were removed in #1141 - // when all 6 were consolidated into the single `memory_tree` dispatcher. + // Verify all superseded tool names are gone. for old_name in [ + "memory_tree", + "query_memory", + "memory_recall", "memory_tree_search_entities", "memory_tree_query_topic", "memory_tree_query_source", @@ -183,7 +182,7 @@ fn orchestrator_lists_memory_tree_tools() { .any(|line| line == entry || line == entry_comma); assert!( !old_tool_present, - "orchestrator agent.toml must NOT list '{old_name}' — removed in #1141 (use 'memory_tree' with mode= dispatch)" + "orchestrator agent.toml must NOT list '{old_name}' — superseded by 'call_memory_agent'" ); } } diff --git a/tests/orchestrator_presentation_wiring.rs b/tests/orchestrator_presentation_wiring.rs index a52953891..cb68a3a01 100644 --- a/tests/orchestrator_presentation_wiring.rs +++ b/tests/orchestrator_presentation_wiring.rs @@ -49,7 +49,7 @@ fn presentation_agent_lists_generate_presentation_and_grounding_tools() { lists_named_tool(PRESENTATION_AGENT_TOML, TOOL_NAME), "presentation_agent must list '{TOOL_NAME}'" ); - for grounding_tool in ["memory_tree", "query_memory", "web_search_tool"] { + for grounding_tool in ["call_memory_agent", "web_search_tool"] { assert!( lists_named_tool(PRESENTATION_AGENT_TOML, grounding_tool), "presentation_agent must list grounding tool '{grounding_tool}'"