fix(memory): stop autosaving assistant summaries and filter legacy entries
This commit is contained in:
parent
6d745e9cb3
commit
d714d3984e
8 changed files with 173 additions and 34 deletions
|
|
@ -129,13 +129,17 @@ Notes:
|
|||
| Key | Default | Purpose |
|
||||
|---|---|---|
|
||||
| `backend` | `sqlite` | `sqlite`, `lucid`, `markdown`, `none` |
|
||||
| `auto_save` | `true` | automatic persistence |
|
||||
| `auto_save` | `true` | persist user-stated inputs only (assistant outputs are excluded) |
|
||||
| `embedding_provider` | `none` | `none`, `openai`, or custom endpoint |
|
||||
| `embedding_model` | `text-embedding-3-small` | embedding model ID, or `hint:<name>` route |
|
||||
| `embedding_dimensions` | `1536` | expected vector size for selected embedding model |
|
||||
| `vector_weight` | `0.7` | hybrid ranking vector weight |
|
||||
| `keyword_weight` | `0.3` | hybrid ranking keyword weight |
|
||||
|
||||
Notes:
|
||||
|
||||
- Memory context injection ignores legacy `assistant_resp*` auto-save keys to prevent old model-authored summaries from being treated as facts.
|
||||
|
||||
## `[[model_routes]]` and `[[embedding_routes]]`
|
||||
|
||||
Use route hints so integrations can keep stable names while model IDs evolve.
|
||||
|
|
|
|||
|
|
@ -10,7 +10,6 @@ use crate::providers::{self, ChatMessage, ChatRequest, ConversationMessage, Prov
|
|||
use crate::runtime;
|
||||
use crate::security::SecurityPolicy;
|
||||
use crate::tools::{self, Tool, ToolSpec};
|
||||
use crate::util::truncate_with_ellipsis;
|
||||
use anyhow::Result;
|
||||
use std::io::Write as IoWrite;
|
||||
use std::sync::Arc;
|
||||
|
|
@ -487,14 +486,6 @@ impl Agent {
|
|||
)));
|
||||
self.trim_history();
|
||||
|
||||
if self.auto_save {
|
||||
let summary = truncate_with_ellipsis(&final_text, 100);
|
||||
let _ = self
|
||||
.memory
|
||||
.store("assistant_resp", &summary, MemoryCategory::Daily, None)
|
||||
.await;
|
||||
}
|
||||
|
||||
return Ok(final_text);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -226,9 +226,16 @@ async fn build_context(mem: &dyn Memory, user_msg: &str, min_relevance_score: f6
|
|||
if !relevant.is_empty() {
|
||||
context.push_str("[Memory context]\n");
|
||||
for entry in &relevant {
|
||||
if memory::is_assistant_autosave_key(&entry.key) {
|
||||
continue;
|
||||
}
|
||||
let _ = writeln!(context, "- {}: {}", entry.key, entry.content);
|
||||
}
|
||||
context.push('\n');
|
||||
if context != "[Memory context]\n" {
|
||||
context.push('\n');
|
||||
} else {
|
||||
context.clear();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1433,15 +1440,6 @@ pub async fn run(
|
|||
final_output = response.clone();
|
||||
println!("{response}");
|
||||
observer.record_event(&ObserverEvent::TurnComplete);
|
||||
|
||||
// Auto-save assistant response to daily log
|
||||
if config.memory.auto_save {
|
||||
let summary = truncate_with_ellipsis(&response, 100);
|
||||
let response_key = autosave_memory_key("assistant_resp");
|
||||
let _ = mem
|
||||
.store(&response_key, &summary, MemoryCategory::Daily, None)
|
||||
.await;
|
||||
}
|
||||
} else {
|
||||
println!("🦀 ZeroClaw Interactive Mode");
|
||||
println!("Type /help for commands.\n");
|
||||
|
|
@ -1591,14 +1589,6 @@ pub async fn run(
|
|||
|
||||
// Hard cap as a safety net.
|
||||
trim_history(&mut history, config.agent.max_history_messages);
|
||||
|
||||
if config.memory.auto_save {
|
||||
let summary = truncate_with_ellipsis(&response, 100);
|
||||
let response_key = autosave_memory_key("assistant_resp");
|
||||
let _ = mem
|
||||
.store(&response_key, &summary, MemoryCategory::Daily, None)
|
||||
.await;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -2441,6 +2431,33 @@ Done."#;
|
|||
assert!(recalled.iter().any(|entry| entry.content.contains("45")));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn build_context_ignores_legacy_assistant_autosave_entries() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let mem = SqliteMemory::new(tmp.path()).unwrap();
|
||||
mem.store(
|
||||
"assistant_resp_poisoned",
|
||||
"User suffered a fabricated event",
|
||||
MemoryCategory::Daily,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
mem.store(
|
||||
"user_msg_real",
|
||||
"User asked for concise status updates",
|
||||
MemoryCategory::Conversation,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let context = build_context(&mem, "status updates", 0.0).await;
|
||||
assert!(context.contains("user_msg_real"));
|
||||
assert!(!context.contains("assistant_resp_poisoned"));
|
||||
assert!(!context.contains("fabricated event"));
|
||||
}
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════
|
||||
// Recovery Tests - Tool Call Parsing Edge Cases
|
||||
// ═══════════════════════════════════════════════════════════════════════
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
use crate::memory::Memory;
|
||||
use crate::memory::{self, Memory};
|
||||
use async_trait::async_trait;
|
||||
use std::fmt::Write;
|
||||
|
||||
|
|
@ -45,6 +45,9 @@ impl MemoryLoader for DefaultMemoryLoader {
|
|||
|
||||
let mut context = String::from("[Memory context]\n");
|
||||
for entry in entries {
|
||||
if memory::is_assistant_autosave_key(&entry.key) {
|
||||
continue;
|
||||
}
|
||||
if let Some(score) = entry.score {
|
||||
if score < self.min_relevance_score {
|
||||
continue;
|
||||
|
|
@ -67,8 +70,12 @@ impl MemoryLoader for DefaultMemoryLoader {
|
|||
mod tests {
|
||||
use super::*;
|
||||
use crate::memory::{Memory, MemoryCategory, MemoryEntry};
|
||||
use std::sync::Arc;
|
||||
|
||||
struct MockMemory;
|
||||
struct MockMemoryWithEntries {
|
||||
entries: Arc<Vec<MemoryEntry>>,
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl Memory for MockMemory {
|
||||
|
|
@ -131,6 +138,56 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl Memory for MockMemoryWithEntries {
|
||||
async fn store(
|
||||
&self,
|
||||
_key: &str,
|
||||
_content: &str,
|
||||
_category: MemoryCategory,
|
||||
_session_id: Option<&str>,
|
||||
) -> anyhow::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn recall(
|
||||
&self,
|
||||
_query: &str,
|
||||
_limit: usize,
|
||||
_session_id: Option<&str>,
|
||||
) -> anyhow::Result<Vec<MemoryEntry>> {
|
||||
Ok(self.entries.as_ref().clone())
|
||||
}
|
||||
|
||||
async fn get(&self, _key: &str) -> anyhow::Result<Option<MemoryEntry>> {
|
||||
Ok(None)
|
||||
}
|
||||
|
||||
async fn list(
|
||||
&self,
|
||||
_category: Option<&MemoryCategory>,
|
||||
_session_id: Option<&str>,
|
||||
) -> anyhow::Result<Vec<MemoryEntry>> {
|
||||
Ok(vec![])
|
||||
}
|
||||
|
||||
async fn forget(&self, _key: &str) -> anyhow::Result<bool> {
|
||||
Ok(true)
|
||||
}
|
||||
|
||||
async fn count(&self) -> anyhow::Result<usize> {
|
||||
Ok(self.entries.len())
|
||||
}
|
||||
|
||||
async fn health_check(&self) -> bool {
|
||||
true
|
||||
}
|
||||
|
||||
fn name(&self) -> &str {
|
||||
"mock-with-entries"
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn default_loader_formats_context() {
|
||||
let loader = DefaultMemoryLoader::default();
|
||||
|
|
@ -138,4 +195,36 @@ mod tests {
|
|||
assert!(context.contains("[Memory context]"));
|
||||
assert!(context.contains("- k: v"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn default_loader_skips_legacy_assistant_autosave_entries() {
|
||||
let loader = DefaultMemoryLoader::new(5, 0.0);
|
||||
let memory = MockMemoryWithEntries {
|
||||
entries: Arc::new(vec![
|
||||
MemoryEntry {
|
||||
id: "1".into(),
|
||||
key: "assistant_resp_legacy".into(),
|
||||
content: "fabricated detail".into(),
|
||||
category: MemoryCategory::Daily,
|
||||
timestamp: "now".into(),
|
||||
session_id: None,
|
||||
score: Some(0.95),
|
||||
},
|
||||
MemoryEntry {
|
||||
id: "2".into(),
|
||||
key: "user_fact".into(),
|
||||
content: "User prefers concise answers".into(),
|
||||
category: MemoryCategory::Conversation,
|
||||
timestamp: "now".into(),
|
||||
session_id: None,
|
||||
score: Some(0.9),
|
||||
},
|
||||
]),
|
||||
};
|
||||
|
||||
let context = loader.load_context(&memory, "answer style").await.unwrap();
|
||||
assert!(context.contains("user_fact"));
|
||||
assert!(!context.contains("assistant_resp_legacy"));
|
||||
assert!(!context.contains("fabricated detail"));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -624,7 +624,7 @@ async fn history_trims_after_max_messages() {
|
|||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
|
||||
#[tokio::test]
|
||||
async fn auto_save_stores_messages_in_memory() {
|
||||
async fn auto_save_stores_only_user_messages_in_memory() {
|
||||
let (mem, _tmp) = make_sqlite_memory();
|
||||
let provider = Box::new(ScriptedProvider::new(vec![text_response(
|
||||
"I remember everything",
|
||||
|
|
@ -639,11 +639,25 @@ async fn auto_save_stores_messages_in_memory() {
|
|||
|
||||
let _ = agent.turn("Remember this fact").await.unwrap();
|
||||
|
||||
// Both user message and assistant response should be saved
|
||||
// Auto-save only persists user-stated input, never assistant-generated summaries.
|
||||
let count = mem.count().await.unwrap();
|
||||
assert_eq!(
|
||||
count, 1,
|
||||
"Expected exactly 1 user memory entry, got {count}"
|
||||
);
|
||||
|
||||
let stored = mem.get("user_msg").await.unwrap();
|
||||
assert!(stored.is_some(), "Expected user_msg key to be present");
|
||||
assert_eq!(
|
||||
stored.unwrap().content,
|
||||
"Remember this fact",
|
||||
"Stored memory should match the original user message"
|
||||
);
|
||||
|
||||
let assistant = mem.get("assistant_resp").await.unwrap();
|
||||
assert!(
|
||||
count >= 2,
|
||||
"Expected at least 2 memory entries, got {count}"
|
||||
assistant.is_none(),
|
||||
"assistant_resp should not be auto-saved anymore"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -293,6 +293,10 @@ fn compact_sender_history(ctx: &ChannelRuntimeContext, sender_key: &str) -> bool
|
|||
}
|
||||
|
||||
fn should_skip_memory_context_entry(key: &str, content: &str) -> bool {
|
||||
if memory::is_assistant_autosave_key(key) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if key.trim().to_ascii_lowercase().ends_with("_history") {
|
||||
return true;
|
||||
}
|
||||
|
|
@ -2167,6 +2171,10 @@ mod tests {
|
|||
"telegram_123_history",
|
||||
r#"[{"role":"user"}]"#
|
||||
));
|
||||
assert!(should_skip_memory_context_entry(
|
||||
"assistant_resp_legacy",
|
||||
"fabricated memory"
|
||||
));
|
||||
assert!(!should_skip_memory_context_entry("telegram_123_45", "hi"));
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1387,7 +1387,7 @@ pub struct MemoryConfig {
|
|||
///
|
||||
/// `postgres` requires `[storage.provider.config]` with `db_url` (`dbURL` alias supported).
|
||||
pub backend: String,
|
||||
/// Auto-save conversation context to memory
|
||||
/// Auto-save user-stated conversation input to memory (assistant output is excluded)
|
||||
pub auto_save: bool,
|
||||
/// Run memory/session hygiene (archiving + retention cleanup)
|
||||
#[serde(default = "default_hygiene_enabled")]
|
||||
|
|
|
|||
|
|
@ -75,6 +75,13 @@ pub fn effective_memory_backend_name(
|
|||
memory_backend.trim().to_ascii_lowercase()
|
||||
}
|
||||
|
||||
/// Legacy auto-save key used for model-authored assistant summaries.
|
||||
/// These entries are treated as untrusted context and should not be re-injected.
|
||||
pub fn is_assistant_autosave_key(key: &str) -> bool {
|
||||
let normalized = key.trim().to_ascii_lowercase();
|
||||
normalized == "assistant_resp" || normalized.starts_with("assistant_resp_")
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
struct ResolvedEmbeddingConfig {
|
||||
provider: String,
|
||||
|
|
@ -343,6 +350,15 @@ mod tests {
|
|||
assert_eq!(mem.name(), "sqlite");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn assistant_autosave_key_detection_matches_legacy_patterns() {
|
||||
assert!(is_assistant_autosave_key("assistant_resp"));
|
||||
assert!(is_assistant_autosave_key("assistant_resp_1234"));
|
||||
assert!(is_assistant_autosave_key("ASSISTANT_RESP_abcd"));
|
||||
assert!(!is_assistant_autosave_key("assistant_response"));
|
||||
assert!(!is_assistant_autosave_key("user_msg_1234"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn factory_markdown() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue