From 0d7a53d1126a134de105e3b05cdb884b042f8ec5 Mon Sep 17 00:00:00 2001 From: coderloganli Date: Mon, 17 Aug 2026 21:50:49 -0400 Subject: [PATCH 1/2] Remember the caller between calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agent forgot everything on hanging up: history was the last six turns of the current session, so a companion met the same person new each time. Long-term semantic memory only. A bounded set of natural-language facts about the caller — a category and one sentence — injected whole into the prompt and never searched. Retrieval was the obvious answer and does not fit: it runs inside the turn against a two-second budget, and it fires on similarity when what a companion must know is unconditional. Reasoning in ADR-0021. Reading costs one indexed local query on the turn path and no network call. Writing is a model call every N turns on a spawned task the turn never waits for (ADR-0022); a failure there leaves the stored set alone and the call unaffected. Facts are keyed on (user_id, character_id), so what one persona was told another does not know (ADR-0023). A caller can see what is held about them and delete it, at GET and DELETE /api/memory. A uid identifies but does not authenticate, so notes kept about a person have to be visible to them. Off unless configured. The [memory] section switches it on and bounds it; an absent section leaves it off, so a configuration written before this does not silently start sending notes to the model provider. CI now runs the agent and api suites, which is what gates any of it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NgGCfYp1iBEpD8iL4Dg5XQ --- .github/workflows/ci.yml | 5 + crates/agent/adapters/postgres.rs | 243 +++++- crates/agent/application/memory.rs | 760 ++++++++++++++++++ crates/agent/application/mod.rs | 730 ++++++++++++++++- crates/agent/domain/mod.rs | 82 +- crates/agent/ports/mod.rs | 74 +- crates/agent/src/lib.rs | 13 +- crates/api/src/lib.rs | 1 + crates/api/src/memory.rs | 321 ++++++++ crates/api/src/router.rs | 36 +- crates/app/src/bootstrap.rs | 28 + crates/app/src/lib.rs | 1 + crates/app/src/llm_config.rs | 26 +- crates/app/src/memory.rs | 54 ++ crates/app/src/prompts.rs | 3 +- crates/app/tests/settings.rs | 1 + crates/config/src/lib.rs | 92 +++ .../20260817_000001_agent_memory_facts.sql | 27 + ...memory-is-extracted-facts-not-retrieval.md | 79 ++ ...emory-extraction-runs-off-the-turn-path.md | 61 ++ ...-memory-is-scoped-to-caller-and-persona.md | 43 + docs/adr/README.md | 5 +- docs/architecture.md | 28 +- docs/product.md | 29 +- sonari.toml.example | 51 ++ 25 files changed, 2764 insertions(+), 29 deletions(-) create mode 100644 crates/agent/application/memory.rs create mode 100644 crates/api/src/memory.rs create mode 100644 crates/app/src/memory.rs create mode 100644 crates/platform/postgres/migrations/20260817_000001_agent_memory_facts.sql create mode 100644 docs/adr/0021-memory-is-extracted-facts-not-retrieval.md create mode 100644 docs/adr/0022-memory-extraction-runs-off-the-turn-path.md create mode 100644 docs/adr/0023-memory-is-scoped-to-caller-and-persona.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 85284d6..c845366 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,6 +36,11 @@ jobs: # need credentials or a running stack skip themselves. - name: Test the evaluation harness run: cargo test -p harness + # The conversation core and the HTTP surface. Both are already compiled by + # the step above, so this costs one link each, and neither needs a database + # or a key: the tests that would run against Postgres skip themselves. + - name: Test the conversation core and the HTTP surface + run: cargo test -p agent -p api compose: runs-on: ubuntu-latest diff --git a/crates/agent/adapters/postgres.rs b/crates/agent/adapters/postgres.rs index b21af4f..b69ee85 100644 --- a/crates/agent/adapters/postgres.rs +++ b/crates/agent/adapters/postgres.rs @@ -2,9 +2,10 @@ use crate::{ AgentArchiveMessage, AgentCallerIdentity, AgentMessage, AgentMessageArchive, AgentSession, LlmUsageLog, LlmUsageStats, MessageRole, PartnerConversationPromptOverride, PromptTemplate, PromptTemplateKey, ProviderKey, SessionUsageSummary, + domain::{ExtractedFact, MemoryCategory, MemoryFact}, ports::{ AgentArchiveRepository, AgentMessageRepository, AgentSessionRepository, - AgentSettingsRepository, PartnerConversationPromptOverrideRepository, + AgentSettingsRepository, MemoryStore, PartnerConversationPromptOverrideRepository, PromptTemplateRepository, UsageLogRepository, }, }; @@ -53,6 +54,12 @@ pub struct PostgresAgentSettingsRepository { pool: PgPool, } +/// Where a caller's facts live (ADR-0021). Sets in, sets out. +#[derive(Debug, Clone)] +pub struct PostgresMemoryStore { + pool: PgPool, +} + macro_rules! repo_new { ($name:ident) => { impl $name { @@ -70,6 +77,7 @@ repo_new!(PostgresAgentMessageRepository); repo_new!(PostgresAgentArchiveRepository); repo_new!(PostgresAgentUsageLogRepository); repo_new!(PostgresAgentSettingsRepository); +repo_new!(PostgresMemoryStore); fn map_sqlx_error(err: sqlx::Error) -> AppError { // 这里把 sqlx 错误收敛为 shared-kernel 统一错误模型。 @@ -529,3 +537,236 @@ impl AgentSettingsRepository for PostgresAgentSettingsRepository { Ok(()) } } + +/// Reads one row. A category the database holds but the code does not know is +/// treated as a corrupt row rather than guessed at; the check constraint means +/// it cannot happen without a migration going wrong. +fn memory_fact_from_row(row: &sqlx::postgres::PgRow) -> AppResult { + let raw: String = row.get("category"); + let category = MemoryCategory::parse(&raw) + .ok_or_else(|| AppError::internal(format!("unknown memory category: {raw}")))?; + Ok(MemoryFact { + user_id: row.get("user_id"), + character_id: row.get("character_id"), + category, + content: row.get("content"), + first_seen_at: row.get("first_seen_at"), + updated_at: row.get("updated_at"), + source_session_id: row.get("source_session_id"), + }) +} + +#[async_trait] +impl MemoryStore for PostgresMemoryStore { + async fn load(&self, user_id: i64, character_id: i64) -> AppResult> { + // Ordered so the injected text is the same from one turn to the next: + // an ordering the database picks would reshuffle the prompt for no + // reason. + let rows = sqlx::query( + "select user_id, character_id, category, content, first_seen_at, updated_at, source_session_id from agent_memory_facts where user_id = $1 and character_id = $2 order by first_seen_at, id", + ) + .bind(user_id) + .bind(character_id) + .fetch_all(&self.pool) + .await + .map_err(map_sqlx_error)?; + rows.iter().map(memory_fact_from_row).collect() + } + + async fn load_all(&self, user_id: i64) -> AppResult> { + let rows = sqlx::query( + "select user_id, character_id, category, content, first_seen_at, updated_at, source_session_id from agent_memory_facts where user_id = $1 order by character_id, first_seen_at, id", + ) + .bind(user_id) + .fetch_all(&self.pool) + .await + .map_err(map_sqlx_error)?; + rows.iter().map(memory_fact_from_row).collect() + } + + async fn replace( + &self, + user_id: i64, + character_id: i64, + source_session_id: &str, + facts: &[ExtractedFact], + ) -> AppResult<()> { + // One transaction, because a half-applied set is a set nobody asked for: + // the caller would be remembered as someone they partly are. + let mut tx = self.pool.begin().await.map_err(map_sqlx_error)?; + let contents: Vec = facts.iter().map(|fact| fact.content.clone()).collect(); + + sqlx::query( + "delete from agent_memory_facts where user_id = $1 and character_id = $2 and content <> all($3)", + ) + .bind(user_id) + .bind(character_id) + .bind(&contents) + .execute(&mut *tx) + .await + .map_err(map_sqlx_error)?; + + for fact in facts { + // A fact still true keeps when it was first learned; only the + // confirmation is new. The unique constraint is what makes this one + // statement instead of a read and a branch. + sqlx::query( + "insert into agent_memory_facts (user_id, character_id, category, content, first_seen_at, updated_at, source_session_id) values ($1, $2, $3, $4, now(), now(), $5) on conflict (user_id, character_id, content) do update set category = excluded.category, updated_at = excluded.updated_at, source_session_id = excluded.source_session_id", + ) + .bind(user_id) + .bind(character_id) + .bind(fact.category.as_str()) + .bind(&fact.content) + .bind(source_session_id) + .execute(&mut *tx) + .await + .map_err(map_sqlx_error)?; + } + + tx.commit().await.map_err(map_sqlx_error) + } + + async fn delete(&self, user_id: i64, character_id: Option) -> AppResult { + let result = match character_id { + Some(character_id) => { + sqlx::query( + "delete from agent_memory_facts where user_id = $1 and character_id = $2", + ) + .bind(user_id) + .bind(character_id) + .execute(&self.pool) + .await + } + None => { + sqlx::query("delete from agent_memory_facts where user_id = $1") + .bind(user_id) + .execute(&self.pool) + .await + } + }; + Ok(result.map_err(map_sqlx_error)?.rows_affected()) + } +} + +#[cfg(test)] +mod memory_store_tests { + //! Test case 26 of task.md — the reconcile SQL. + //! + //! Skips itself without `DATABASE_DSN`, as the harness tests that need + //! credentials do. Everything else about memory runs against fakes; this is + //! the one thing a fake cannot check, because the behaviour under test is + //! three statements in one transaction. + + use super::*; + use crate::domain::MemoryCategory; + + async fn pool_or_skip() -> Option { + let dsn = std::env::var("DATABASE_DSN").ok()?; + match PgPool::connect(&dsn).await { + Ok(pool) => Some(pool), + Err(error) => { + eprintln!("skipping: cannot reach the database: {error}"); + None + } + } + } + + fn extracted(category: MemoryCategory, content: &str) -> ExtractedFact { + ExtractedFact { + category, + content: content.to_owned(), + } + } + + /// Test case 26 — `replace` preserves, inserts and deletes. + #[tokio::test] + async fn replace_preserves_inserts_and_deletes() { + let Some(pool) = pool_or_skip().await else { + return; + }; + let store = PostgresMemoryStore::new(pool); + // A user id no call would produce, so a run cannot disturb real rows. + let user_id = -4242; + let character_id = 11; + store + .delete(user_id, None) + .await + .expect("clear anything left by an earlier run"); + + store + .replace( + user_id, + character_id, + "session-a", + &[ + extracted(MemoryCategory::Identity, "The caller is called Ada."), + extracted(MemoryCategory::Situation, "The caller is job hunting."), + extracted( + MemoryCategory::Preference, + "The caller dislikes small talk.", + ), + ], + ) + .await + .expect("write the first set"); + let first = store + .load(user_id, character_id) + .await + .expect("read the first set"); + assert_eq!(first.len(), 3); + let kept_before = first + .iter() + .find(|fact| fact.content == "The caller is called Ada.") + .expect("the kept fact") + .clone(); + + store + .replace( + user_id, + character_id, + "session-b", + &[ + extracted(MemoryCategory::Identity, "The caller is called Ada."), + extracted(MemoryCategory::Situation, "The caller starts on Monday."), + ], + ) + .await + .expect("write the replacement set"); + let second = store + .load(user_id, character_id) + .await + .expect("read the replacement set"); + + assert_eq!(second.len(), 2, "the absent fact is deleted"); + let kept_after = second + .iter() + .find(|fact| fact.content == "The caller is called Ada.") + .expect("the kept fact survives"); + assert_eq!( + kept_after.first_seen_at, kept_before.first_seen_at, + "a fact that is still true keeps when it was first learned" + ); + assert!( + kept_after.updated_at >= kept_before.updated_at, + "and is confirmed again" + ); + assert_eq!(kept_after.source_session_id, "session-b"); + assert!( + second + .iter() + .any(|fact| fact.content == "The caller starts on Monday."), + "the new fact is inserted" + ); + assert!( + !second + .iter() + .any(|fact| fact.content == "The caller is job hunting."), + "the superseded fact is gone" + ); + + store + .delete(user_id, None) + .await + .expect("leave nothing behind"); + } +} diff --git a/crates/agent/application/memory.rs b/crates/agent/application/memory.rs new file mode 100644 index 0000000..e753f57 --- /dev/null +++ b/crates/agent/application/memory.rs @@ -0,0 +1,760 @@ +//! What the agent knows about a caller, and how it learns it. +//! +//! Two halves that never run at the same time. Reading is a local query on the +//! turn path; writing is a model call on a task the turn path spawned and forgot +//! about (ADR-0022). Between them sits a bounded set of natural-language facts, +//! injected whole rather than searched (ADR-0021). + +use std::sync::Arc; + +use async_trait::async_trait; +use shared_kernel::{AppError, AppResult}; + +use crate::domain::{ + AgentCallerIdentity, AgentMessage, ExtractedFact, MemoryCategory, MemoryFact, MessageRole, + PromptTemplateKey, ProviderKey, +}; +use crate::ports::{ + AgentMessageRepository, AgentSessionRepository, Clock, LlmCompletionRequest, LlmGateway, + LlmProviderConfigRepository, LlmRequestMessage, MemoryStore, PromptTemplateRepository, +}; + +/// How memory behaves. All of it from `sonari.toml`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MemoryPolicy { + pub enabled: bool, + /// Completed turns between extractions. + pub extract_every_turns: i32, + pub max_facts: usize, + pub max_facts_per_category: usize, +} + +impl Default for MemoryPolicy { + fn default() -> Self { + Self { + enabled: false, + extract_every_turns: 4, + max_facts: 40, + max_facts_per_category: 12, + } + } +} + +/// How the fact set is introduced to the model. Deliberately short: the persona +/// prompts are what shape the reply, and this is context, not instruction. +const MEMORY_PREAMBLE: &str = concat!( + "What you already know about the person you are speaking to, from earlier ", + "calls. Use it the way a friend would — do not recite it, and do not ", + "mention that you have notes." +); + +/// Renders a fact set into the one system message that carries it. +/// +/// `None` for an empty set: an agent with nothing to remember sends exactly the +/// prompt it sent before this existed. +pub fn render_facts(facts: &[MemoryFact]) -> Option { + if facts.is_empty() { + return None; + } + let mut rendered = String::from(MEMORY_PREAMBLE); + // Grouped in the order the category list declares, so the stable things are + // read before the passing ones, and the text does not reshuffle between + // turns. + for category in MemoryCategory::ALL { + let mut in_category = facts + .iter() + .filter(|fact| fact.category == category) + .peekable(); + if in_category.peek().is_none() { + continue; + } + rendered.push_str("\n\n"); + rendered.push_str(category.as_str()); + rendered.push(':'); + for fact in in_category { + rendered.push_str("\n- "); + rendered.push_str(fact.content.trim()); + } + } + Some(rendered) +} + +/// Trims a model's output down to what may be stored. +/// +/// Pure, and where both caps live. Returns what survives; the caller logs what +/// did not. Order is the model's, because it puts what it thought mattered +/// first and the caps cut from the end. +pub fn validate(facts: Vec, policy: &MemoryPolicy) -> Vec { + let mut seen: Vec = Vec::new(); + let mut per_category: Vec<(MemoryCategory, usize)> = Vec::new(); + let mut kept = Vec::new(); + for fact in facts { + let content = fact.content.trim(); + if content.is_empty() { + continue; + } + // Content alone, not content and category. The storage key is + // `(user_id, character_id, content)`, so the same sentence filed under + // two categories is one row either way — deduping by the pair here would + // only move the collision into the upsert, where the later row silently + // rewrites the earlier one's category. Case and surrounding space are + // not a difference worth a row: the model restates the same sentence + // more than one way. + let fingerprint = content.to_lowercase(); + if seen.contains(&fingerprint) { + continue; + } + let count = match per_category + .iter_mut() + .find(|(category, _)| *category == fact.category) + { + Some((_, count)) => count, + None => { + per_category.push((fact.category, 0)); + &mut per_category.last_mut().expect("just pushed").1 + } + }; + if *count >= policy.max_facts_per_category { + continue; + } + *count += 1; + seen.push(fingerprint); + kept.push(ExtractedFact { + category: fact.category, + content: content.to_owned(), + }); + if kept.len() == policy.max_facts { + break; + } + } + kept +} + +/// What the extraction model is asked to return. +#[derive(Debug, serde::Deserialize)] +struct ExtractionReply { + facts: Vec, +} + +#[derive(Debug, serde::Deserialize)] +struct ExtractionReplyFact { + category: String, + content: String, +} + +/// What a reply yielded, and how much of it was unusable. +struct ParsedReply { + facts: Vec, + /// Facts named with a category outside the closed list. Counted rather than + /// discarded silently: if the prompt or the model drifts into inventing + /// categories, the only way anyone finds out is this number. + unknown_categories: usize, +} + +/// Reads the fact set out of a reply. +/// +/// `None` when there is no object to read, which is the model answering in prose +/// instead of JSON. A fact whose category is not in the closed list is dropped +/// rather than the reply rejected wholesale: one bad row is not a reason to lose +/// the other four. +fn parse_reply(reply: &str) -> Option { + // Models wrap JSON in prose or a fence often enough that finding the object + // is worth more than insisting the whole reply is one. + let start = reply.find('{')?; + let end = reply.rfind('}')?; + if end <= start { + return None; + } + let parsed: ExtractionReply = serde_json::from_str(&reply[start..=end]).ok()?; + let mut facts = Vec::new(); + let mut unknown_categories = 0; + for fact in parsed.facts { + match MemoryCategory::parse(fact.category.trim()) { + Some(category) => facts.push(ExtractedFact { + category, + content: fact.content, + }), + None => unknown_categories += 1, + } + } + Some(ParsedReply { + facts, + unknown_categories, + }) +} + +/// What the extraction model is shown: what is already known, and what was just +/// said. Not a prompt — the instruction is the template's job. +fn render_extraction_input(current: &[MemoryFact], recent: &[AgentMessage]) -> String { + let mut input = String::from("Known so far:"); + if current.is_empty() { + input.push_str("\n(nothing)"); + } else { + for fact in current { + input.push_str("\n- "); + input.push_str(fact.category.as_str()); + input.push_str(": "); + input.push_str(&fact.content); + } + } + input.push_str("\n\nThe conversation since:"); + for message in recent { + input.push('\n'); + input.push_str(match message.role { + MessageRole::Assistant => "agent: ", + _ => "caller: ", + }); + input.push_str(message.content.trim()); + } + input +} + +/// Reads and forgets, for the caller's own routes. No writing: a caller may see +/// and delete what is held about them, not author it. +#[async_trait] +pub trait MemoryUseCases: Send + Sync { + async fn list(&self, user_id: i64) -> AppResult>; + async fn forget(&self, user_id: i64, character_id: Option) -> AppResult; +} + +pub struct MemoryDependencies { + pub memory: Arc, + pub sessions: Arc, + pub messages: Arc, + pub providers: Arc, + pub templates: Arc, + pub gateway: Arc, + pub clock: Arc, + pub policy: MemoryPolicy, +} + +/// Turns conversation into facts. +/// +/// Trait objects rather than the type parameters `AgentService` uses: this is +/// constructed once in the composition root and handed to a spawned task, where +/// a dozen type parameters buy nothing. +pub struct MemoryService { + deps: MemoryDependencies, +} + +impl MemoryService { + pub fn new(deps: MemoryDependencies) -> Self { + Self { deps } + } + + /// Reads the recent turns and the current set, asks for a replacement, and + /// writes it. + /// + /// Returns nothing, including on failure. Its caller is a spawned task with + /// nowhere to return an error to, and a failed extraction means the agent + /// learns nothing this time — not that anything is wrong with the call + /// (ADR-0022). + pub async fn extract(&self, session_id: &str) { + if let Err(error) = self.try_extract(session_id).await { + tracing::warn!(session_id, %error, "memory extraction failed"); + } + } + + async fn try_extract(&self, session_id: &str) -> AppResult<()> { + let session = self + .deps + .sessions + .get_by_id(session_id) + .await? + .ok_or_else(|| AppError::not_found("agent session not found"))?; + let AgentCallerIdentity::PlatformUser { user_id } = session.caller; + let character_id = session.character_id; + + let current = self.deps.memory.load(user_id, character_id).await?; + let recent = self + .deps + .messages + .list_recent(session_id, self.deps.policy.extract_every_turns) + .await?; + if recent.is_empty() { + return Ok(()); + } + + let provider = self + .deps + .providers + .get_by_key(ProviderKey::Assistant) + .await? + .ok_or_else(|| AppError::invalid_input("no model is configured for extraction"))?; + let template = self + .deps + .templates + .get_by_key(PromptTemplateKey::MemoryExtraction) + .await? + .ok_or_else(|| AppError::invalid_input("no extraction prompt is configured"))?; + + let request = LlmCompletionRequest { + endpoint_url: provider.endpoint_url.clone(), + api_key: provider.api_key.clone(), + model_name: provider.model_name.clone(), + temperature: provider.temperature, + frequency_penalty: provider.frequency_penalty, + messages: vec![ + LlmRequestMessage { + role: MessageRole::System.as_str().to_owned(), + content: self.render_instruction(&template.template_text), + }, + LlmRequestMessage { + role: MessageRole::User.as_str().to_owned(), + content: render_extraction_input(¤t, &recent), + }, + ], + max_tokens: None, + tools: Vec::new(), + }; + + let reply = super::collect_reply(self.deps.gateway.stream(request).await?).await?; + let Some(parsed) = parse_reply(&reply.content) else { + tracing::warn!( + session_id, + "extraction reply was not a fact set; the stored set is left alone" + ); + return Ok(()); + }; + // Everything the model put forward, including what it named with a + // category that does not exist, so the drop count is the whole truth. + let offered = parsed.facts.len() + parsed.unknown_categories; + let facts = validate(parsed.facts, &self.deps.policy); + if facts.is_empty() { + // A set that came back empty is far more likely to be a model having + // a bad turn than a caller whose every fact stopped being true, and + // the cost of the two mistakes is not symmetric. + tracing::warn!( + session_id, + offered, + "extraction produced no storable facts; the stored set is left alone" + ); + return Ok(()); + } + + self.deps + .memory + .replace(user_id, character_id, session_id, &facts) + .await?; + tracing::info!( + session_id, + user_id, + character_id, + held_before = current.len(), + offered, + stored = facts.len(), + dropped = offered - facts.len(), + "memory extracted" + ); + Ok(()) + } + + fn render_instruction(&self, template_text: &str) -> String { + let categories = MemoryCategory::ALL + .iter() + .map(|category| category.as_str()) + .collect::>() + .join(", "); + template_text + .replace("{{max_facts}}", &self.deps.policy.max_facts.to_string()) + .replace( + "{{max_facts_per_category}}", + &self.deps.policy.max_facts_per_category.to_string(), + ) + .replace("{{categories}}", &categories) + } +} + +#[async_trait] +impl MemoryUseCases for MemoryService { + async fn list(&self, user_id: i64) -> AppResult> { + self.deps.memory.load_all(user_id).await + } + + async fn forget(&self, user_id: i64, character_id: Option) -> AppResult { + let deleted = self.deps.memory.delete(user_id, character_id).await?; + tracing::info!(user_id, ?character_id, deleted, "memory forgotten"); + Ok(deleted) + } +} + +#[cfg(test)] +mod tests { + //! Test cases 11-17 of task.md. Case 18, the acceptance case, needs both + //! halves and lives beside the injection tests in `application/mod.rs`. + //! + //! The gateway is scripted, so nothing here judges what a model would + //! actually extract. What is under test is that a reply becomes the stored + //! set, that the caps and the closed category list hold, and that every way + //! the extraction can fail leaves the stored set alone. + + use super::*; + use crate::domain::{ + AgentCallerIdentity, AgentMessage, AgentSession, LlmProviderConfig, MemoryCategory, + MessageRole, PromptTemplate, PromptTemplateKey, ProviderKey, + }; + use crate::ports::{LlmCompletionRequest, LlmDelta, LlmStream, LlmUsage}; + use shared_kernel::AppError; + use std::sync::Mutex; + + /// Records every set it was asked to store, and serves a fixed one. + #[derive(Default)] + struct SpyMemory { + held: Vec, + written: Mutex>>, + } + + impl SpyMemory { + fn holding(held: Vec) -> Self { + Self { + held, + written: Mutex::new(Vec::new()), + } + } + + fn last_written(&self) -> Vec { + let written = self.written.lock().unwrap(); + assert_eq!(written.len(), 1, "expected exactly one write"); + written[0].clone() + } + + fn never_written(&self) -> bool { + self.written.lock().unwrap().is_empty() + } + } + + #[async_trait] + impl MemoryStore for SpyMemory { + async fn load(&self, _user_id: i64, _character_id: i64) -> AppResult> { + Ok(self.held.clone()) + } + async fn load_all(&self, _user_id: i64) -> AppResult> { + Ok(self.held.clone()) + } + async fn replace( + &self, + _user_id: i64, + _character_id: i64, + _source_session_id: &str, + facts: &[ExtractedFact], + ) -> AppResult<()> { + self.written.lock().unwrap().push(facts.to_vec()); + Ok(()) + } + async fn delete(&self, _user_id: i64, _character_id: Option) -> AppResult { + Ok(0) + } + } + + struct ScriptedGateway { + reply: Option, + } + + impl ScriptedGateway { + fn saying(reply: &str) -> Self { + Self { + reply: Some(reply.to_owned()), + } + } + + fn broken() -> Self { + Self { reply: None } + } + } + + #[async_trait] + impl LlmGateway for ScriptedGateway { + async fn stream(&self, _request: LlmCompletionRequest) -> AppResult { + match &self.reply { + Some(reply) => Ok(Box::pin(futures::stream::iter(vec![ + Ok(LlmDelta::Token(reply.clone())), + Ok(LlmDelta::Done(LlmUsage::default())), + ]))), + None => Err(AppError::unavailable("the model endpoint is unreachable")), + } + } + } + + struct StubSessions; + + #[async_trait] + impl AgentSessionRepository for StubSessions { + async fn create(&self, session: &AgentSession) -> AppResult { + Ok(session.clone()) + } + async fn get_by_id(&self, session_id: &str) -> AppResult> { + Ok(Some(AgentSession { + id: session_id.to_owned(), + caller: AgentCallerIdentity::PlatformUser { user_id: 7 }, + character_id: 11, + timezone: "UTC".into(), + scene_id: None, + started_at: chrono::Utc::now(), + ended_at: None, + })) + } + async fn end(&self, _session_id: &str) -> AppResult<()> { + Ok(()) + } + } + + struct StubMessages; + + #[async_trait] + impl AgentMessageRepository for StubMessages { + async fn append(&self, message: &AgentMessage) -> AppResult { + Ok(message.clone()) + } + async fn list_recent( + &self, + session_id: &str, + _recent_turns: i32, + ) -> AppResult> { + Ok(vec![AgentMessage { + id: 1, + session_id: session_id.to_owned(), + role: MessageRole::User, + content: "I have a cat called Coal.".into(), + turn_number: 1, + created_at: chrono::Utc::now(), + }]) + } + async fn list_all(&self, _session_id: &str) -> AppResult> { + Ok(Vec::new()) + } + async fn next_turn_number(&self, _session_id: &str) -> AppResult { + Ok(2) + } + } + + struct StubProviders; + + #[async_trait] + impl LlmProviderConfigRepository for StubProviders { + async fn get_by_key( + &self, + provider_key: ProviderKey, + ) -> AppResult> { + Ok(Some(LlmProviderConfig { + provider_key, + endpoint_url: "https://example.invalid".into(), + api_key: String::new(), + model_name: "extraction-model".into(), + temperature: 0.0, + frequency_penalty: 0.0, + updated_at: chrono::Utc::now(), + })) + } + async fn list_all(&self) -> AppResult> { + Ok(Vec::new()) + } + async fn upsert(&self, config: &LlmProviderConfig) -> AppResult { + Ok(config.clone()) + } + } + + struct StubTemplates; + + #[async_trait] + impl PromptTemplateRepository for StubTemplates { + async fn get_by_key(&self, key: PromptTemplateKey) -> AppResult> { + Ok(Some(PromptTemplate { + id: 1, + template_key: key, + template_text: "Extract at most {{max_facts}} facts.".into(), + updated_at: chrono::Utc::now(), + })) + } + async fn list_all(&self) -> AppResult> { + Ok(Vec::new()) + } + async fn upsert(&self, template: &PromptTemplate) -> AppResult { + Ok(template.clone()) + } + } + + struct StubClock; + + impl Clock for StubClock { + fn now(&self) -> chrono::DateTime { + chrono::Utc::now() + } + } + + fn policy() -> MemoryPolicy { + MemoryPolicy { + enabled: true, + extract_every_turns: 4, + max_facts: 40, + max_facts_per_category: 12, + } + } + + fn service_with( + store: Arc, + gateway: ScriptedGateway, + policy: MemoryPolicy, + ) -> MemoryService { + MemoryService::new(MemoryDependencies { + memory: store, + sessions: Arc::new(StubSessions), + messages: Arc::new(StubMessages), + providers: Arc::new(StubProviders), + templates: Arc::new(StubTemplates), + gateway: Arc::new(gateway), + clock: Arc::new(StubClock), + policy, + }) + } + + fn extracted(category: MemoryCategory, content: &str) -> ExtractedFact { + ExtractedFact { + category, + content: content.to_owned(), + } + } + + /// Test case 11 — a reply becomes the stored set. + #[tokio::test] + async fn a_reply_becomes_the_stored_set() { + let store = Arc::new(SpyMemory::default()); + let reply = r#"{"facts":[ + {"category":"relationship","content":"The caller has a cat called Coal."}, + {"category":"identity","content":"The caller is called Ada."} + ]}"#; + let service = service_with(store.clone(), ScriptedGateway::saying(reply), policy()); + + service.extract("session-1").await; + + assert_eq!( + store.last_written(), + vec![ + extracted( + MemoryCategory::Relationship, + "The caller has a cat called Coal." + ), + extracted(MemoryCategory::Identity, "The caller is called Ada."), + ] + ); + } + + /// Test case 12 — unknown categories are dropped, the rest kept. + #[tokio::test] + async fn unknown_categories_are_dropped() { + let store = Arc::new(SpyMemory::default()); + let reply = r#"{"facts":[ + {"category":"identity","content":"The caller is called Ada."}, + {"category":"favourite_colour","content":"The caller likes green."}, + {"category":"preference","content":"The caller dislikes small talk."}, + {"category":"situation","content":"The caller has an interview on Friday."} + ]}"#; + let service = service_with(store.clone(), ScriptedGateway::saying(reply), policy()); + + service.extract("session-1").await; + + let written = store.last_written(); + assert_eq!(written.len(), 3); + assert!(!written.iter().any(|f| f.content.contains("green"))); + } + + /// Test case 13 — both caps hold. + #[test] + fn caps_hold() { + let per_category = MemoryPolicy { + max_facts_per_category: 2, + ..policy() + }; + let four_situations: Vec = (1..=4) + .map(|n| extracted(MemoryCategory::Situation, &format!("Situation {n}."))) + .collect(); + + assert_eq!(validate(four_situations, &per_category).len(), 2); + + let total = MemoryPolicy { + max_facts: 3, + ..policy() + }; + let five_across = vec![ + extracted(MemoryCategory::Identity, "One."), + extracted(MemoryCategory::Relationship, "Two."), + extracted(MemoryCategory::Preference, "Three."), + extracted(MemoryCategory::Situation, "Four."), + extracted(MemoryCategory::Commitment, "Five."), + ]; + + assert_eq!(validate(five_across, &total).len(), 3); + } + + /// Test case 14 — duplicates collapse, whatever category they arrive under. + #[test] + fn duplicates_collapse() { + let facts = vec![ + extracted(MemoryCategory::Identity, "The caller is called Ada."), + extracted(MemoryCategory::Identity, "the caller is called ada. "), + ]; + + assert_eq!(validate(facts, &policy()).len(), 1); + + // The storage key is the content, so the same sentence under two + // categories is one row whatever happens here. Collapsing it now keeps + // the decision where it can be seen, rather than in whichever upsert + // happens to run second. + let across_categories = vec![ + extracted(MemoryCategory::Situation, "The caller is job hunting."), + extracted(MemoryCategory::Preference, "The caller is job hunting."), + ]; + let kept = validate(across_categories, &policy()); + + assert_eq!(kept.len(), 1); + assert_eq!(kept[0].category, MemoryCategory::Situation); + } + + /// Test case 15 — unparseable output changes nothing. + #[tokio::test] + async fn unparseable_output_changes_nothing() { + let store = Arc::new(SpyMemory::holding(vec![MemoryFact { + user_id: 7, + character_id: 11, + category: MemoryCategory::Identity, + content: "The caller is called Ada.".into(), + first_seen_at: chrono::Utc::now(), + updated_at: chrono::Utc::now(), + source_session_id: "earlier".into(), + }])); + let service = service_with( + store.clone(), + ScriptedGateway::saying("Sure! Here is what I learned about them:"), + policy(), + ); + + service.extract("session-1").await; + + assert!(store.never_written()); + } + + /// Test case 16 — a gateway failure changes nothing. + #[tokio::test] + async fn a_gateway_failure_changes_nothing() { + let store = Arc::new(SpyMemory::default()); + let service = service_with(store.clone(), ScriptedGateway::broken(), policy()); + + service.extract("session-1").await; + + assert!(store.never_written()); + } + + /// Test case 17 — an empty extracted set does not wipe memory. + #[tokio::test] + async fn an_empty_extracted_set_does_not_wipe_memory() { + let store = Arc::new(SpyMemory::default()); + let service = service_with( + store.clone(), + ScriptedGateway::saying(r#"{"facts":[]}"#), + policy(), + ); + + service.extract("session-1").await; + + assert!(store.never_written()); + } +} diff --git a/crates/agent/application/mod.rs b/crates/agent/application/mod.rs index 6d4a588..b2292be 100644 --- a/crates/agent/application/mod.rs +++ b/crates/agent/application/mod.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; + use async_trait::async_trait; use character_context::{CharacterPromptContext, CharacterPromptContextReadPort}; use serde::Serialize; @@ -10,10 +12,16 @@ use crate::domain::{ use crate::ports::{ AgentCallControlPort, AgentMessageRepository, AgentSessionRepository, AgentSettingsRepository, Clock, CreateAgentSessionRequest, CreateAgentSessionResult, IdGenerator, LlmCompletionRequest, - LlmGateway, LlmProviderConfigRepository, LlmRequestMessage, - PartnerConversationPromptOverrideRepository, PromptTemplateRepository, UsageLogRepository, + LlmGateway, LlmProviderConfigRepository, LlmRequestMessage, MemoryExtractionScheduler, + MemoryStore, PartnerConversationPromptOverrideRepository, PromptTemplateRepository, + UsageLogRepository, }; +#[path = "memory.rs"] +pub mod memory; + +pub use memory::{MemoryDependencies, MemoryPolicy, MemoryService, MemoryUseCases, render_facts}; + const DEFAULT_RECENT_TURNS: i32 = 6; #[derive(Debug, Clone)] @@ -114,6 +122,9 @@ pub struct AgentDependencies { pub ids: I, pub clock: K, pub settings: Box, + pub memory: Arc, + pub extraction: Arc, + pub memory_policy: MemoryPolicy, } pub struct AgentService { @@ -128,6 +139,9 @@ pub struct AgentService { ids: I, clock: K, settings: Box, + memory: Arc, + extraction: Arc, + memory_policy: MemoryPolicy, } impl AgentService { @@ -144,6 +158,9 @@ impl AgentService ids: deps.ids, clock: deps.clock, settings: deps.settings, + memory: deps.memory, + extraction: deps.extraction, + memory_policy: deps.memory_policy, } } } @@ -225,6 +242,7 @@ where let mut messages = self .build_system_messages(&prompt_context, &session.timezone, None) .await?; + messages.extend(self.build_memory_message(&session).await); messages.push(LlmRequestMessage { role: MessageRole::User.as_str().to_owned(), content: user_prompt, @@ -293,6 +311,14 @@ where turn_number, ) .await?; + // ADR-0022: the turn schedules and moves on. The modulo is here, in the + // layer that can be tested, and only the spawning is in the adapter. + if self.memory_policy.enabled + && self.memory_policy.extract_every_turns > 0 + && turn_number % self.memory_policy.extract_every_turns == 0 + { + self.extraction.schedule(&session.id); + } Ok(ChatOutcome { reply_text: response.content, first_token_at_ms: response.first_token_at_ms, @@ -306,7 +332,7 @@ where /// Callers that need audio as it is produced consume the stream directly; this /// is for the paths that only want the completed text, and it is where usage /// and tool calls are gathered. -async fn collect_reply( +pub(crate) async fn collect_reply( mut stream: crate::ports::LlmStream, ) -> AppResult { use futures::StreamExt; @@ -580,6 +606,7 @@ where let mut messages = self .build_system_messages(&prompt_context, &session.timezone, None) .await?; + messages.extend(self.build_memory_message(session).await); messages.extend( self.messages .list_recent(&session.id, recent_turns) @@ -597,6 +624,34 @@ where Ok(messages) } + /// The one system message carrying what is known about this caller, or + /// nothing. + /// + /// Never fails a turn. A store that is down makes the agent forgetful, which + /// is a far smaller thing than a call that does not answer, so the error is + /// logged and the turn goes on without it. + async fn build_memory_message(&self, session: &AgentSession) -> Option { + if !self.memory_policy.enabled { + return None; + } + let AgentCallerIdentity::PlatformUser { user_id } = session.caller; + let facts = match self.memory.load(user_id, session.character_id).await { + Ok(facts) => facts, + Err(error) => { + tracing::warn!( + session_id = %session.id, + %error, + "could not read what is remembered; the turn continues without it" + ); + return None; + } + }; + memory::render_facts(&facts).map(|content| LlmRequestMessage { + role: MessageRole::System.as_str().to_owned(), + content, + }) + } + async fn build_system_messages( &self, prompt_context: &CharacterPromptContext, @@ -1064,6 +1119,9 @@ mod tests { ids: StubIds, clock: StubClock, settings: Box::new(StubSettings), + memory: Arc::new(RecordingMemory::default()), + extraction: Arc::new(RecordingScheduler::default()), + memory_policy: MemoryPolicy::default(), }); let prompt_context = CharacterPromptContext { character: character_context::CharacterPromptProfile { @@ -1115,6 +1173,9 @@ mod tests { ids: StubIds, clock: StubClock, settings: Box::new(StubSettings), + memory: Arc::new(RecordingMemory::default()), + extraction: Arc::new(RecordingScheduler::default()), + memory_policy: MemoryPolicy::default(), }); let result = service @@ -1128,4 +1189,667 @@ mod tests { assert!(result.is_err()); } + // ---- Long-term memory ------------------------------------------------- + // + // Test cases 1-10 of task.md. The fakes below record what they were asked + // for, because most of what matters here is a question asked of the store or + // the scheduler, not a value returned to the caller. + + use crate::domain::{MemoryCategory, MemoryFact}; + use crate::ports::{MemoryExtractionScheduler, MemoryStore}; + use std::sync::Mutex; + + /// A store holding a fixed set, remembering every key it was asked for. + #[derive(Default)] + struct RecordingMemory { + facts: Vec, + asked: Mutex>, + } + + impl RecordingMemory { + fn holding(facts: Vec) -> Self { + Self { + facts, + asked: Mutex::new(Vec::new()), + } + } + } + + #[async_trait] + impl MemoryStore for RecordingMemory { + async fn load(&self, user_id: i64, character_id: i64) -> AppResult> { + self.asked.lock().unwrap().push((user_id, character_id)); + Ok(self + .facts + .iter() + .filter(|fact| fact.user_id == user_id && fact.character_id == character_id) + .cloned() + .collect()) + } + async fn load_all(&self, _user_id: i64) -> AppResult> { + Ok(self.facts.clone()) + } + async fn replace( + &self, + _user_id: i64, + _character_id: i64, + _source_session_id: &str, + _facts: &[crate::domain::ExtractedFact], + ) -> AppResult<()> { + Ok(()) + } + async fn delete(&self, _user_id: i64, _character_id: Option) -> AppResult { + Ok(0) + } + } + + /// A store that is down. + #[derive(Default)] + struct BrokenMemory; + + #[async_trait] + impl MemoryStore for BrokenMemory { + async fn load(&self, _user_id: i64, _character_id: i64) -> AppResult> { + Err(AppError::internal("memory is unavailable")) + } + async fn load_all(&self, _user_id: i64) -> AppResult> { + Err(AppError::internal("memory is unavailable")) + } + async fn replace( + &self, + _user_id: i64, + _character_id: i64, + _source_session_id: &str, + _facts: &[crate::domain::ExtractedFact], + ) -> AppResult<()> { + Err(AppError::internal("memory is unavailable")) + } + async fn delete(&self, _user_id: i64, _character_id: Option) -> AppResult { + Err(AppError::internal("memory is unavailable")) + } + } + + #[derive(Default)] + struct RecordingScheduler { + scheduled: Mutex>, + } + + impl MemoryExtractionScheduler for RecordingScheduler { + fn schedule(&self, session_id: &str) { + self.scheduled.lock().unwrap().push(session_id.to_owned()); + } + } + + /// Captures the messages the model was sent, which is where injection is + /// visible. The reply is fixed: what it says is not what these tests are + /// about. + #[derive(Default)] + struct CapturingGateway { + requests: Mutex>, + } + + #[async_trait] + impl LlmGateway for CapturingGateway { + async fn stream( + &self, + request: LlmCompletionRequest, + ) -> AppResult { + use crate::ports::{LlmDelta, LlmUsage}; + self.requests.lock().unwrap().push(request); + Ok(Box::pin(futures::stream::iter(vec![ + Ok(LlmDelta::Token("mm.".into())), + Ok(LlmDelta::Done(LlmUsage::default())), + ]))) + } + } + + /// Lets the test hold the gateway it handed to the service. `LlmGateway` has + /// no blanket impl for `Arc`, and giving it one would apply to the whole + /// crate for the sake of a test. + struct SharedGateway(Arc); + + #[async_trait] + impl LlmGateway for SharedGateway { + async fn stream( + &self, + request: LlmCompletionRequest, + ) -> AppResult { + self.0.stream(request).await + } + } + + /// The session every memory test runs against: caller 7, persona 11. + #[derive(Default)] + struct SessionFor7And11; + + #[async_trait] + impl AgentSessionRepository for SessionFor7And11 { + async fn create(&self, session: &AgentSession) -> AppResult { + Ok(session.clone()) + } + async fn get_by_id(&self, session_id: &str) -> AppResult> { + Ok(Some(AgentSession { + id: session_id.to_owned(), + caller: AgentCallerIdentity::PlatformUser { user_id: 7 }, + character_id: 11, + timezone: "UTC".into(), + scene_id: None, + started_at: chrono::Utc::now(), + ended_at: None, + })) + } + async fn end(&self, _session_id: &str) -> AppResult<()> { + Ok(()) + } + } + + /// Two turns of history, so the memory message has something to sit before. + #[derive(Default)] + struct MessagesWithHistory { + next_turn: i32, + } + + #[async_trait] + impl AgentMessageRepository for MessagesWithHistory { + async fn append(&self, message: &AgentMessage) -> AppResult { + Ok(message.clone()) + } + async fn list_recent( + &self, + session_id: &str, + _recent_turns: i32, + ) -> AppResult> { + Ok(vec![AgentMessage { + id: 1, + session_id: session_id.to_owned(), + role: MessageRole::User, + content: "an earlier thing the caller said".into(), + turn_number: 1, + created_at: chrono::Utc::now(), + }]) + } + async fn list_all(&self, _session_id: &str) -> AppResult> { + Ok(Vec::new()) + } + async fn next_turn_number(&self, _session_id: &str) -> AppResult { + Ok(self.next_turn) + } + } + + fn fact( + user_id: i64, + character_id: i64, + category: MemoryCategory, + content: &str, + ) -> MemoryFact { + MemoryFact { + user_id, + character_id, + category, + content: content.to_owned(), + first_seen_at: chrono::Utc::now(), + updated_at: chrono::Utc::now(), + source_session_id: "earlier-session".into(), + } + } + + fn three_facts() -> Vec { + vec![ + fact(7, 11, MemoryCategory::Identity, "The caller is called Ada."), + fact( + 7, + 11, + MemoryCategory::Relationship, + "The caller has a cat called Coal.", + ), + fact( + 7, + 11, + MemoryCategory::Commitment, + "The agent said it would ask how the interview went.", + ), + ] + } + + struct MemoryHarness { + service: AgentService< + StubProviders, + StubTemplates, + StubPartnerPromptOverrides, + SessionFor7And11, + MessagesWithHistory, + StubUsage, + SharedGateway, + StubCharacters, + StubIds, + StubClock, + >, + gateway: Arc, + scheduler: Arc, + } + + fn harness_with( + memory: Arc, + policy: MemoryPolicy, + next_turn: i32, + ) -> MemoryHarness { + let scheduler = Arc::new(RecordingScheduler::default()); + harness_from(memory, policy, next_turn, scheduler.clone(), scheduler) + } + + /// The same, with the scheduler chosen by the caller — case 18 needs one + /// that really runs the extraction. The recording scheduler still exists so + /// the harness has one to hold; nothing asserts on it in that case. + fn harness_with_scheduler( + memory: Arc, + policy: MemoryPolicy, + next_turn: i32, + extraction: Arc, + ) -> MemoryHarness { + harness_from( + memory, + policy, + next_turn, + extraction, + Arc::new(RecordingScheduler::default()), + ) + } + + fn harness_from( + memory: Arc, + policy: MemoryPolicy, + next_turn: i32, + extraction: Arc, + scheduler: Arc, + ) -> MemoryHarness { + let gateway = Arc::new(CapturingGateway::default()); + let service = AgentService::new(AgentDependencies { + providers: StubProviders, + templates: StubTemplates, + partner_prompt_overrides: StubPartnerPromptOverrides, + sessions: SessionFor7And11, + messages: MessagesWithHistory { next_turn }, + usage_logs: StubUsage, + gateway: SharedGateway(gateway.clone()), + characters: StubCharacters, + ids: StubIds, + clock: StubClock, + settings: Box::new(StubSettings), + memory, + extraction, + memory_policy: policy, + }); + MemoryHarness { + service, + gateway, + scheduler, + } + } + + impl MemoryHarness { + /// `chat_once` is on two traits the service implements, so a bare method + /// call is ambiguous. Naming the trait once here keeps every test + /// reading as a turn rather than as a disambiguation. + async fn turn_in(&self, session_id: &str, message: &str) -> AppResult { + AgentUseCases::chat_once( + &self.service, + ChatCommand { + session_id: session_id.to_owned(), + user_message: message.to_owned(), + }, + ) + .await + } + + async fn turn(&self, message: &str) -> AppResult { + self.turn_in("session-1", message).await + } + + async fn welcome(&self) -> AppResult { + AgentUseCases::generate_welcome_message(&self.service, "session-1").await + } + } + + fn enabled_policy() -> MemoryPolicy { + MemoryPolicy { + enabled: true, + extract_every_turns: 4, + max_facts: 40, + max_facts_per_category: 12, + } + } + + /// The messages the model was actually sent on the only call made. + fn sent_messages(gateway: &CapturingGateway) -> Vec { + let requests = gateway.requests.lock().unwrap(); + assert_eq!(requests.len(), 1, "expected exactly one model call"); + requests[0].messages.clone() + } + + fn memory_message(messages: &[LlmRequestMessage]) -> Option<(usize, String)> { + messages.iter().enumerate().find_map(|(index, message)| { + (message.role == MessageRole::System.as_str() && message.content.contains("Coal")) + .then(|| (index, message.content.clone())) + }) + } + + /// Test case 1 — a non-empty fact set renders. + #[test] + fn a_non_empty_fact_set_renders() { + let rendered = render_facts(&three_facts()).expect("a non-empty set renders"); + + assert!(rendered.contains("The caller is called Ada.")); + assert!(rendered.contains("The caller has a cat called Coal.")); + assert!(rendered.contains("The agent said it would ask how the interview went.")); + let identity = rendered.find("identity").expect("identity heading"); + let relationship = rendered.find("relationship").expect("relationship heading"); + let commitment = rendered.find("commitment").expect("commitment heading"); + assert!( + identity < relationship && relationship < commitment, + "categories render in the order MemoryCategory::ALL declares" + ); + } + + /// Test case 2 — an empty fact set renders to nothing, and the prompt is + /// exactly what it was before memory existed. + #[tokio::test] + async fn an_empty_fact_set_renders_to_nothing() { + assert_eq!(render_facts(&[]), None); + + let harness = harness_with(Arc::new(RecordingMemory::default()), enabled_policy(), 1); + harness.turn("hello").await.expect("the turn succeeds"); + + let messages = sent_messages(&harness.gateway); + assert_eq!( + messages.iter().filter(|m| m.role == "system").count(), + 3, + "the three persona system messages and nothing else" + ); + } + + /// Test case 3 — the memory message sits after the persona and before the + /// history. + #[tokio::test] + async fn the_memory_message_sits_between_the_persona_and_the_history() { + let harness = harness_with( + Arc::new(RecordingMemory::holding(three_facts())), + enabled_policy(), + 1, + ); + harness.turn("hello").await.expect("the turn succeeds"); + + let messages = sent_messages(&harness.gateway); + let (index, _) = memory_message(&messages).expect("the memory message is sent"); + assert_eq!(index, 3, "after the three persona system messages"); + let first_history = messages + .iter() + .position(|m| m.content == "an earlier thing the caller said") + .expect("the history is sent"); + assert!(index < first_history, "and before the history"); + } + + /// Test case 4 — a memory read failure does not fail the turn. + #[tokio::test] + async fn a_memory_read_failure_does_not_fail_the_turn() { + let harness = harness_with(Arc::new(BrokenMemory), enabled_policy(), 1); + + let outcome = harness + .turn("hello") + .await + .expect("a broken store does not fail the call"); + + assert_eq!(outcome.reply_text, "mm."); + assert!(memory_message(&sent_messages(&harness.gateway)).is_none()); + } + + /// Test case 5 — the welcome message carries the facts. + #[tokio::test] + async fn the_welcome_message_carries_the_facts() { + let harness = harness_with( + Arc::new(RecordingMemory::holding(three_facts())), + enabled_policy(), + 1, + ); + + harness + .welcome() + .await + .expect("the welcome message is produced"); + + assert!(memory_message(&sent_messages(&harness.gateway)).is_some()); + } + + /// Test case 6 — the set is read for this caller and this persona. + #[tokio::test] + async fn the_set_is_read_for_this_caller_and_this_persona() { + let store = Arc::new(RecordingMemory::holding(three_facts())); + let harness = harness_with(store.clone(), enabled_policy(), 1); + + harness.turn("hello").await.expect("the turn succeeds"); + + assert_eq!(store.asked.lock().unwrap().as_slice(), [(7, 11)]); + } + + /// Test case 7 — another persona sees nothing. + #[tokio::test] + async fn another_persona_sees_nothing() { + let facts = vec![fact( + 7, + 12, + MemoryCategory::Relationship, + "The caller has a cat called Coal.", + )]; + let harness = harness_with( + Arc::new(RecordingMemory::holding(facts)), + enabled_policy(), + 1, + ); + + harness.turn("hello").await.expect("the turn succeeds"); + + assert!(memory_message(&sent_messages(&harness.gateway)).is_none()); + } + + /// Test case 8 — extraction is scheduled on the Nth turn. + #[tokio::test] + async fn extraction_is_scheduled_on_the_nth_turn() { + let harness = harness_with(Arc::new(RecordingMemory::default()), enabled_policy(), 4); + + harness.turn("hello").await.expect("the turn succeeds"); + + assert_eq!( + harness.scheduler.scheduled.lock().unwrap().as_slice(), + ["session-1".to_owned()] + ); + } + + /// Test case 9 — it is not scheduled on other turns. + #[tokio::test] + async fn extraction_is_not_scheduled_on_other_turns() { + for turn in [1, 2, 3, 5] { + let harness = + harness_with(Arc::new(RecordingMemory::default()), enabled_policy(), turn); + + harness.turn("hello").await.expect("the turn succeeds"); + + assert!( + harness.scheduler.scheduled.lock().unwrap().is_empty(), + "turn {turn} scheduled an extraction" + ); + } + } + + /// Test case 10 — disabled means never. + #[tokio::test] + async fn disabled_memory_never_schedules() { + let policy = MemoryPolicy { + enabled: false, + ..enabled_policy() + }; + let harness = harness_with(Arc::new(RecordingMemory::default()), policy, 4); + + harness.turn("hello").await.expect("the turn succeeds"); + + assert!(harness.scheduler.scheduled.lock().unwrap().is_empty()); + } + + /// A store that actually keeps what it is given, so extraction and injection + /// can be run against the same one. + #[derive(Default)] + struct InMemoryStore { + facts: Mutex>, + } + + #[async_trait] + impl MemoryStore for InMemoryStore { + async fn load(&self, user_id: i64, character_id: i64) -> AppResult> { + Ok(self + .facts + .lock() + .unwrap() + .iter() + .filter(|fact| fact.user_id == user_id && fact.character_id == character_id) + .cloned() + .collect()) + } + async fn load_all(&self, user_id: i64) -> AppResult> { + Ok(self + .facts + .lock() + .unwrap() + .iter() + .filter(|fact| fact.user_id == user_id) + .cloned() + .collect()) + } + async fn replace( + &self, + user_id: i64, + character_id: i64, + source_session_id: &str, + facts: &[crate::domain::ExtractedFact], + ) -> AppResult<()> { + let mut held = self.facts.lock().unwrap(); + held.retain(|fact| !(fact.user_id == user_id && fact.character_id == character_id)); + let now = chrono::Utc::now(); + held.extend(facts.iter().map(|fact| MemoryFact { + user_id, + character_id, + category: fact.category, + content: fact.content.clone(), + first_seen_at: now, + updated_at: now, + source_session_id: source_session_id.to_owned(), + })); + Ok(()) + } + async fn delete(&self, user_id: i64, character_id: Option) -> AppResult { + let mut held = self.facts.lock().unwrap(); + let before = held.len(); + held.retain(|fact| { + !(fact.user_id == user_id + && character_id + .map(|id| fact.character_id == id) + .unwrap_or(true)) + }); + Ok((before - held.len()) as u64) + } + } + + /// Test case 18 — a fact survives the call. + /// + /// Driven through the turn path rather than by calling extraction directly: + /// session A is an ordinary turn, and everything after it — the scheduling + /// decision, the scheduler spawning, the extraction, the write — happens the + /// way it happens in a call. Session B is a second session for the same + /// caller and persona. Asserted on the request the gateway received, because + /// what the model replies is whatever the fake was told to say, so the reply + /// proves nothing and the prompt proves everything. + #[tokio::test] + async fn a_fact_survives_the_call() { + let store = Arc::new(InMemoryStore::default()); + let extraction = Arc::new(MemoryService::new(MemoryDependencies { + memory: store.clone(), + sessions: Arc::new(SessionFor7And11), + messages: Arc::new(MessagesWithHistory { next_turn: 4 }), + providers: Arc::new(StubProviders), + templates: Arc::new(StubTemplates), + gateway: Arc::new(FactExtractingGateway), + clock: Arc::new(StubClock), + policy: enabled_policy(), + })); + let scheduler = Arc::new(SpawningTestScheduler::new(extraction)); + + // Session A: a turn, on the turn the policy says to extract. + let session_a = + harness_with_scheduler(store.clone(), enabled_policy(), 4, scheduler.clone()); + session_a + .turn_in("session-a", "I have a cat called Coal") + .await + .expect("the turn succeeds"); + scheduler.settle().await; + + // Session B: an entirely new session for the same caller and persona. + let session_b = harness_with(store.clone(), enabled_policy(), 1); + session_b + .turn_in("session-b", "how have you been") + .await + .expect("the turn succeeds"); + + let (_, content) = memory_message(&sent_messages(&session_b.gateway)) + .expect("what session A learned reaches session B's prompt"); + assert!(content.contains("Coal")); + } + + /// Spawns like the composition root's scheduler does, and hands the test a + /// way to wait for what it spawned. A test that did the extraction itself + /// would pass even if the turn path never scheduled anything. + struct SpawningTestScheduler { + memory: Arc, + spawned: Mutex>>, + } + + impl SpawningTestScheduler { + fn new(memory: Arc) -> Self { + Self { + memory, + spawned: Mutex::new(Vec::new()), + } + } + + async fn settle(&self) { + let handles: Vec<_> = self.spawned.lock().unwrap().drain(..).collect(); + for handle in handles { + handle.await.expect("the extraction task finished"); + } + } + } + + impl MemoryExtractionScheduler for SpawningTestScheduler { + fn schedule(&self, session_id: &str) { + let memory = self.memory.clone(); + let session_id = session_id.to_owned(); + self.spawned.lock().unwrap().push(tokio::spawn(async move { + memory.extract(&session_id).await + })); + } + } + + /// Returns the one fact session A is supposed to learn. + struct FactExtractingGateway; + + #[async_trait] + impl LlmGateway for FactExtractingGateway { + async fn stream( + &self, + _request: LlmCompletionRequest, + ) -> AppResult { + use crate::ports::{LlmDelta, LlmUsage}; + let reply = r#"{"facts":[{"category":"relationship","content":"The caller has a cat called Coal."}]}"#; + Ok(Box::pin(futures::stream::iter(vec![ + Ok(LlmDelta::Token(reply.into())), + Ok(LlmDelta::Done(LlmUsage::default())), + ]))) + } + } } diff --git a/crates/agent/domain/mod.rs b/crates/agent/domain/mod.rs index f3616b3..14a9e56 100644 --- a/crates/agent/domain/mod.rs +++ b/crates/agent/domain/mod.rs @@ -35,7 +35,7 @@ pub enum PromptTemplateKey { ConversationSystem2, ConversationSystem3, ConversationWelcomeUser, - AssistantSystem, + MemoryExtraction, } impl PromptTemplateKey { @@ -45,7 +45,7 @@ impl PromptTemplateKey { "conversation_system_2" => Some(Self::ConversationSystem2), "conversation_system_3" => Some(Self::ConversationSystem3), "conversation_welcome_user" => Some(Self::ConversationWelcomeUser), - "assistant_system" => Some(Self::AssistantSystem), + "memory_extraction" => Some(Self::MemoryExtraction), _ => None, } } @@ -56,7 +56,7 @@ impl PromptTemplateKey { Self::ConversationSystem2 => "conversation_system_2", Self::ConversationSystem3 => "conversation_system_3", Self::ConversationWelcomeUser => "conversation_welcome_user", - Self::AssistantSystem => "assistant_system", + Self::MemoryExtraction => "memory_extraction", } } } @@ -167,6 +167,82 @@ pub struct AgentMessageArchive { pub archived_at: DateTime, } +/// What kind of thing is remembered. A closed list, because eviction needs a +/// quota to be fair about and the injected text is grouped by it (ADR-0021). +/// +/// The order is the order facts are rendered in: what is stable first, what is +/// passing last. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum MemoryCategory { + /// Who the caller is: name, age, where they live, what they do. + Identity, + /// Who is around them: family, friends, pets, colleagues. + Relationship, + /// What they like, dislike, or will not talk about. + Preference, + /// What is going on right now. This is the category that expires. + Situation, + /// What was agreed between them and the agent. + Commitment, +} + +impl MemoryCategory { + /// Every category, in rendering order. + pub const ALL: [Self; 5] = [ + Self::Identity, + Self::Relationship, + Self::Preference, + Self::Situation, + Self::Commitment, + ]; + + pub fn parse(raw: &str) -> Option { + match raw { + "identity" => Some(Self::Identity), + "relationship" => Some(Self::Relationship), + "preference" => Some(Self::Preference), + "situation" => Some(Self::Situation), + "commitment" => Some(Self::Commitment), + _ => None, + } + } + + pub fn as_str(self) -> &'static str { + match self { + Self::Identity => "identity", + Self::Relationship => "relationship", + Self::Preference => "preference", + Self::Situation => "situation", + Self::Commitment => "commitment", + } + } +} + +/// One thing the agent knows about a caller, as stored. +/// +/// The row is structured; the sentence is not. What is worth remembering about a +/// person is an open set, so `content` stays natural language (ADR-0021). +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct MemoryFact { + pub user_id: i64, + pub character_id: i64, + pub category: MemoryCategory, + pub content: String, + /// When this fact was first learned. Survives a rewrite that keeps it. + pub first_seen_at: DateTime, + pub updated_at: DateTime, + /// The session whose extraction last confirmed it. + pub source_session_id: String, +} + +/// A fact as the model produced it, before it is anyone's. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ExtractedFact { + pub category: MemoryCategory, + pub content: String, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] // 记录一次 LLM 调用的用量和错误结果。 pub struct LlmUsageLog { diff --git a/crates/agent/ports/mod.rs b/crates/agent/ports/mod.rs index 73412bd..1670c5b 100644 --- a/crates/agent/ports/mod.rs +++ b/crates/agent/ports/mod.rs @@ -2,9 +2,9 @@ use async_trait::async_trait; use shared_kernel::AppResult; use crate::domain::{ - AgentCallerIdentity, AgentMessage, AgentMessageArchive, AgentSession, LlmProviderConfig, - LlmUsageLog, LlmUsageStats, PartnerConversationPromptOverride, PromptTemplate, - PromptTemplateKey, ProviderKey, SessionUsageSummary, + AgentCallerIdentity, AgentMessage, AgentMessageArchive, AgentSession, ExtractedFact, + LlmProviderConfig, LlmUsageLog, LlmUsageStats, MemoryFact, PartnerConversationPromptOverride, + PromptTemplate, PromptTemplateKey, ProviderKey, SessionUsageSummary, }; #[derive(Debug, Clone, PartialEq, Eq)] @@ -110,6 +110,74 @@ pub trait UsageLogRepository: Send + Sync { async fn summarize_session(&self, session_id: &str) -> AppResult; } +/// Where the fact set lives. Whole sets in, whole sets out: ADR-0021 made the +/// set the unit, so nothing here reads or writes a single fact. +#[async_trait] +pub trait MemoryStore: Send + Sync { + /// The facts one persona knows about one caller. + async fn load(&self, user_id: i64, character_id: i64) -> AppResult>; + /// Everything held about a caller, across personas. For the caller's own + /// reading, not for a prompt. + async fn load_all(&self, user_id: i64) -> AppResult>; + /// Replaces the set in one transaction. A fact whose content is unchanged + /// keeps its `first_seen_at`; one that is absent is deleted. + async fn replace( + &self, + user_id: i64, + character_id: i64, + source_session_id: &str, + facts: &[ExtractedFact], + ) -> AppResult<()>; + /// Forgets one persona's facts, or all of them when `character_id` is + /// `None`. Returns how many rows went. + async fn delete(&self, user_id: i64, character_id: Option) -> AppResult; +} + +#[async_trait] +impl MemoryStore for std::sync::Arc +where + T: MemoryStore + ?Sized, +{ + async fn load(&self, user_id: i64, character_id: i64) -> AppResult> { + (**self).load(user_id, character_id).await + } + async fn load_all(&self, user_id: i64) -> AppResult> { + (**self).load_all(user_id).await + } + async fn replace( + &self, + user_id: i64, + character_id: i64, + source_session_id: &str, + facts: &[ExtractedFact], + ) -> AppResult<()> { + (**self) + .replace(user_id, character_id, source_session_id, facts) + .await + } + async fn delete(&self, user_id: i64, character_id: Option) -> AppResult { + (**self).delete(user_id, character_id).await + } +} + +/// Starts an extraction without waiting for it (ADR-0022). +/// +/// Deliberately not `async`: the turn path calls this and moves on. How the work +/// actually leaves the current task is the composition root's business, and the +/// only place that knows about spawning. +pub trait MemoryExtractionScheduler: Send + Sync { + fn schedule(&self, session_id: &str); +} + +impl MemoryExtractionScheduler for std::sync::Arc +where + T: MemoryExtractionScheduler + ?Sized, +{ + fn schedule(&self, session_id: &str) { + (**self).schedule(session_id) + } +} + #[async_trait] pub trait AgentSettingsRepository: Send + Sync { async fn get_recent_turns(&self) -> AppResult; diff --git a/crates/agent/src/lib.rs b/crates/agent/src/lib.rs index 68df3b5..72902d0 100644 --- a/crates/agent/src/lib.rs +++ b/crates/agent/src/lib.rs @@ -9,17 +9,22 @@ pub mod ports; pub use adapters::postgres::{ PostgresAgentArchiveRepository, PostgresAgentMessageRepository, PostgresAgentSessionRepository, - PostgresAgentSettingsRepository, PostgresAgentUsageLogRepository, + PostgresAgentSettingsRepository, PostgresAgentUsageLogRepository, PostgresMemoryStore, PostgresPartnerConversationPromptOverrideRepository, PostgresPromptTemplateRepository, }; pub use application::{ AgentDependencies, AgentRuntimeUseCases, AgentService, ChatCommand, ChatOutcome, + MemoryDependencies, MemoryPolicy, MemoryService, MemoryUseCases, PartnerConversationPromptConfigView, UpdateAdminConfigCommand, UpdatePartnerConversationPromptConfigCommand, }; pub use domain::{ AgentArchiveMessage, AgentCallerIdentity, AgentMessage, AgentMessageArchive, AgentSession, - LlmProviderConfig, LlmUsageLog, LlmUsageStats, MessageRole, PartnerConversationPromptOverride, - PromptTemplate, PromptTemplateKey, ProviderKey, SessionUsageSummary, + ExtractedFact, LlmProviderConfig, LlmUsageLog, LlmUsageStats, MemoryCategory, MemoryFact, + MessageRole, PartnerConversationPromptOverride, PromptTemplate, PromptTemplateKey, ProviderKey, + SessionUsageSummary, +}; +pub use ports::{ + AgentCallControlPort, CreateAgentSessionRequest, CreateAgentSessionResult, + MemoryExtractionScheduler, MemoryStore, }; -pub use ports::{AgentCallControlPort, CreateAgentSessionRequest, CreateAgentSessionResult}; diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index 807ffa0..db53427 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -3,6 +3,7 @@ mod call; mod dev_client; mod error; mod health; +mod memory; mod personas; mod response; mod router; diff --git a/crates/api/src/memory.rs b/crates/api/src/memory.rs new file mode 100644 index 0000000..b02f139 --- /dev/null +++ b/crates/api/src/memory.rs @@ -0,0 +1,321 @@ +//! What the agent remembers about the caller, to the caller. +//! +//! A `uid` identifies but does not authenticate, so what is held about a person +//! has to be visible to them and removable by them — product.md §4 says so +//! plainly rather than leaving it to be discovered. Read and delete only: +//! authoring memory is not a surface anybody asked for. + +use std::sync::Arc; + +use agent::{MemoryFact, MemoryUseCases}; +use auth::ports::TokenService; +use axum::{ + Router, + extract::{Extension, Query, State}, + middleware, + routing::{delete, get}, +}; +use serde::{Deserialize, Serialize}; +use shared_kernel::Claims; + +use crate::{admin_auth::require_user_auth, error::ApiError, response::ok}; + +pub fn build_memory_router( + memory_service: Arc, + token_service: Arc, +) -> Router { + Router::new() + .route("/api/memory", get(list_memory)) + .route("/api/memory", delete(forget_memory)) + .route_layer(middleware::from_fn_with_state( + token_service, + require_user_auth, + )) + .with_state(memory_service) +} + +#[derive(Debug, Deserialize, Default)] +struct ForgetQuery { + /// Narrows the deletion to one persona. Absent means forget everything. + character_id: Option, +} + +/// One fact, as the person it is about sees it. +#[derive(Debug, Serialize)] +struct MemoryFactView { + character_id: i64, + category: &'static str, + content: String, + first_seen_at: chrono::DateTime, + updated_at: chrono::DateTime, +} + +impl From for MemoryFactView { + fn from(fact: MemoryFact) -> Self { + Self { + character_id: fact.character_id, + category: fact.category.as_str(), + content: fact.content, + first_seen_at: fact.first_seen_at, + updated_at: fact.updated_at, + } + } +} + +#[derive(Debug, Serialize)] +struct MemoryListData { + facts: Vec, +} + +#[derive(Debug, Serialize)] +struct ForgottenData { + deleted: u64, +} + +async fn list_memory( + State(memory_service): State>, + Extension(claims): Extension, +) -> Result { + // The caller is the token's subject. Nothing in the request says whose + // memory this is, so nothing in the request can ask for someone else's. + let facts = memory_service.list(claims.subject_id).await?; + Ok(ok(MemoryListData { + facts: facts.into_iter().map(MemoryFactView::from).collect(), + })) +} + +async fn forget_memory( + State(memory_service): State>, + Extension(claims): Extension, + Query(query): Query, +) -> Result { + let deleted = memory_service + .forget(claims.subject_id, query.character_id) + .await?; + Ok(ok(ForgottenData { deleted })) +} + +#[cfg(test)] +mod tests { + //! Test cases 19-25 of task.md. + + use std::sync::{Arc, Mutex}; + + use agent::{MemoryCategory, MemoryFact, MemoryUseCases}; + use async_trait::async_trait; + use auth::ports::{TokenPairView, TokenService}; + use axum::{ + body::Body, + http::{Request, StatusCode}, + }; + use shared_kernel::{AppResult, Claims, Role}; + use tower::ServiceExt; + + use super::build_memory_router; + + /// Two callers, so a handler that ignores the token is visible. + #[derive(Default)] + struct FakeMemory { + listed: Mutex>, + forgotten: Mutex)>>, + } + + fn fact(user_id: i64, character_id: i64, content: &str) -> MemoryFact { + MemoryFact { + user_id, + character_id, + category: MemoryCategory::Relationship, + content: content.to_owned(), + first_seen_at: chrono::Utc::now(), + updated_at: chrono::Utc::now(), + source_session_id: "session-a".into(), + } + } + + fn everything() -> Vec { + vec![ + fact(7, 11, "The caller has a cat called Coal."), + fact(7, 12, "The caller mentioned a brother."), + fact(8, 11, "Someone else entirely."), + ] + } + + #[async_trait] + impl MemoryUseCases for FakeMemory { + async fn list(&self, user_id: i64) -> AppResult> { + self.listed.lock().unwrap().push(user_id); + Ok(everything() + .into_iter() + .filter(|fact| fact.user_id == user_id) + .collect()) + } + + async fn forget(&self, user_id: i64, character_id: Option) -> AppResult { + self.forgotten.lock().unwrap().push((user_id, character_id)); + Ok(everything() + .into_iter() + .filter(|fact| { + fact.user_id == user_id + && character_id + .map(|id| fact.character_id == id) + .unwrap_or(true) + }) + .count() as u64) + } + } + + struct Tokens; + + #[async_trait] + impl TokenService for Tokens { + async fn issue_token_pair( + &self, + _subject_id: i64, + _role: &str, + _permissions: &[String], + ) -> AppResult { + unreachable!() + } + + async fn refresh_token_pair(&self, _refresh_token: &str) -> AppResult { + unreachable!() + } + + async fn validate_access_token(&self, access_token: &str) -> AppResult { + match access_token { + "caller-7" => Ok(Claims { + subject_id: 7, + role: Role::User, + }), + _ => Err(shared_kernel::AppError::unauthorized("bad token")), + } + } + } + + fn request(method: &str, uri: &str, token: Option<&str>) -> Request { + let mut builder = Request::builder().uri(uri).method(method); + if let Some(token) = token { + builder = builder.header("authorization", format!("Bearer {token}")); + } + builder.body(Body::empty()).expect("build request") + } + + async fn body_of(response: axum::response::Response) -> serde_json::Value { + let bytes = axum::body::to_bytes(response.into_body(), 64 * 1024) + .await + .expect("read the body"); + serde_json::from_slice(&bytes).expect("the body is JSON") + } + + /// Test case 19 — reading needs a token. + #[tokio::test] + async fn reading_memory_needs_a_token() { + let memory = Arc::new(FakeMemory::default()); + let router = build_memory_router(memory.clone(), Arc::new(Tokens)); + + let response = router + .oneshot(request("GET", "/api/memory", None)) + .await + .expect("route the request"); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + assert!(memory.listed.lock().unwrap().is_empty()); + } + + /// Test case 20 — deleting needs a token, and an unauthenticated request + /// does not reach the store. + #[tokio::test] + async fn deleting_memory_needs_a_token() { + let memory = Arc::new(FakeMemory::default()); + let router = build_memory_router(memory.clone(), Arc::new(Tokens)); + + let response = router + .oneshot(request("DELETE", "/api/memory", None)) + .await + .expect("route the request"); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + assert!(memory.forgotten.lock().unwrap().is_empty()); + } + + /// Test case 22 — reading returns the caller's facts, across personas. + #[tokio::test] + async fn reading_memory_returns_the_callers_facts() { + let memory = Arc::new(FakeMemory::default()); + let router = build_memory_router(memory, Arc::new(Tokens)); + + let response = router + .oneshot(request("GET", "/api/memory", Some("caller-7"))) + .await + .expect("route the request"); + + assert_eq!(response.status(), StatusCode::OK); + let body = body_of(response).await; + let facts = body["data"]["facts"] + .as_array() + .expect("a facts array") + .clone(); + assert_eq!(facts.len(), 2); + assert_eq!(facts[0]["character_id"], 11); + assert_eq!(facts[0]["category"], "relationship"); + assert_eq!(facts[1]["character_id"], 12); + } + + /// Test case 23 — reading returns only the token's caller, and asks the + /// store for that subject. + #[tokio::test] + async fn reading_memory_returns_only_the_tokens_caller() { + let memory = Arc::new(FakeMemory::default()); + let router = build_memory_router(memory.clone(), Arc::new(Tokens)); + + let response = router + .oneshot(request("GET", "/api/memory", Some("caller-7"))) + .await + .expect("route the request"); + + let body = body_of(response).await; + let serialised = body.to_string(); + assert!( + !serialised.contains("Someone else entirely"), + "another caller's facts must not be returned" + ); + assert_eq!(memory.listed.lock().unwrap().as_slice(), [7]); + } + + /// Test case 24 — deleting forgets everything for the caller, for the + /// token's caller rather than one named in the request. + #[tokio::test] + async fn deleting_memory_forgets_everything_for_the_caller() { + let memory = Arc::new(FakeMemory::default()); + let router = build_memory_router(memory.clone(), Arc::new(Tokens)); + + let response = router + .oneshot(request("DELETE", "/api/memory?user_id=8", Some("caller-7"))) + .await + .expect("route the request"); + + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(body_of(response).await["data"]["deleted"], 2); + assert_eq!(memory.forgotten.lock().unwrap().as_slice(), [(7, None)]); + } + + /// Test case 25 — deleting one persona's facts leaves the others. + #[tokio::test] + async fn deleting_one_persona_leaves_the_others() { + let memory = Arc::new(FakeMemory::default()); + let router = build_memory_router(memory.clone(), Arc::new(Tokens)); + + let response = router + .oneshot(request( + "DELETE", + "/api/memory?character_id=11", + Some("caller-7"), + )) + .await + .expect("route the request"); + + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(body_of(response).await["data"]["deleted"], 1); + assert_eq!(memory.forgotten.lock().unwrap().as_slice(), [(7, Some(11))]); + } +} diff --git a/crates/api/src/router.rs b/crates/api/src/router.rs index 954e232..36807e5 100644 --- a/crates/api/src/router.rs +++ b/crates/api/src/router.rs @@ -1,5 +1,6 @@ use std::sync::Arc; +use agent::MemoryUseCases; use auth::ports::TokenService; use axum::{Router, middleware, routing::get}; use call::{CallLogUseCases, CallUseCases}; @@ -17,6 +18,7 @@ pub struct ModuleServices { pub call_service: Arc, pub call_log_service: Arc, pub persona_catalog: Arc, + pub memory_service: Arc, } pub fn build_router() -> Router { @@ -35,6 +37,10 @@ pub fn build_router_with_modules(services: ModuleServices) -> Router { .merge(build_session_router(services.token_service.clone())) .merge(build_personas_router(services.persona_catalog)) .merge(build_dev_client_router()) + .merge(crate::memory::build_memory_router( + services.memory_service, + services.token_service.clone(), + )) .merge(build_call_router( services.call_service, services.call_log_service, @@ -53,6 +59,7 @@ mod tests { use std::sync::Arc; + use agent::{MemoryFact, MemoryUseCases}; use async_trait::async_trait; use auth::ports::{TokenPairView, TokenService}; use axum::{ @@ -126,6 +133,17 @@ mod tests { } } + #[async_trait] + impl MemoryUseCases for Unused { + async fn list(&self, _user_id: i64) -> AppResult> { + Ok(Vec::new()) + } + + async fn forget(&self, _user_id: i64, _character_id: Option) -> AppResult { + Ok(0) + } + } + #[async_trait] impl CharacterCatalogReadPort for Unused { async fn list_characters(&self) -> AppResult> { @@ -138,15 +156,20 @@ mod tests { } async fn status_of(path: &str) -> StatusCode { + status_of_method("GET", path).await + } + + async fn status_of_method(method: &str, path: &str) -> StatusCode { let router = build_router_with_modules(ModuleServices { token_service: Arc::new(Unused), call_service: Arc::new(Unused), call_log_service: Arc::new(Unused), persona_catalog: Arc::new(Unused), + memory_service: Arc::new(Unused), }); let request = Request::builder() .uri(path) - .method("GET") + .method(method) .body(Body::empty()) .expect("build request"); router @@ -168,4 +191,15 @@ mod tests { async fn the_application_lists_personas_without_a_token() { assert_eq!(status_of("/api/personas").await, StatusCode::OK); } + + /// Test case 21 — the application serves the memory routes. A route tested + /// only against its own router can be absent from the one the binary serves. + #[tokio::test] + async fn the_application_serves_the_memory_routes() { + assert_ne!(status_of("/api/memory").await, StatusCode::NOT_FOUND); + assert_ne!( + status_of_method("DELETE", "/api/memory").await, + StatusCode::NOT_FOUND + ); + } } diff --git a/crates/app/src/bootstrap.rs b/crates/app/src/bootstrap.rs index 2e1f4b5..aaa5ae7 100644 --- a/crates/app/src/bootstrap.rs +++ b/crates/app/src/bootstrap.rs @@ -115,6 +115,30 @@ pub async fn run() -> Result<()> { .start() .await .context("failed to start call event outbox publisher")?; + let memory_store = Arc::new(agent::PostgresMemoryStore::new(pool.clone())); + let memory_policy = { + let configured = &settings.get().memory; + agent::MemoryPolicy { + enabled: configured.enabled, + extract_every_turns: configured.extract_every_turns, + max_facts: configured.max_facts, + max_facts_per_category: configured.max_facts_per_category, + } + }; + let memory_service = Arc::new(agent::MemoryService::new(agent::MemoryDependencies { + memory: memory_store.clone(), + sessions: Arc::new(agent::PostgresAgentSessionRepository::new(pool.clone())), + messages: Arc::new(agent::PostgresAgentMessageRepository::new(pool.clone())), + providers: llm_providers.clone(), + templates: Arc::new(crate::prompts::ConfigPromptTemplates::new(settings.clone())), + gateway: Arc::new(agent::adapters::llm::ReqwestLlmGateway::default()), + clock: Arc::new(SystemClock), + policy: memory_policy.clone(), + })); + let memory_use_cases: Arc = memory_service.clone(); + let extraction_scheduler: Arc = Arc::new( + crate::memory::SpawningExtractionScheduler::new(memory_service.clone()), + ); let agent_service = Arc::new(agent::AgentService::new(agent::AgentDependencies { providers: llm_providers.clone(), templates: Arc::new(crate::prompts::ConfigPromptTemplates::new(settings.clone())), @@ -129,6 +153,9 @@ pub async fn run() -> Result<()> { ids: StaticIdGenerator, clock: SystemClock, settings: Box::new(agent::PostgresAgentSettingsRepository::new(pool.clone())), + memory: memory_store, + extraction: extraction_scheduler, + memory_policy, })); let agent_call_control: Arc = agent_service.clone(); let agent_runtime: Arc = agent_service.clone(); @@ -240,6 +267,7 @@ pub async fn run() -> Result<()> { call_service: call_service.clone(), call_log_service, persona_catalog, + memory_service: memory_use_cases, }); tracing::info!("services assembled"); diff --git a/crates/app/src/lib.rs b/crates/app/src/lib.rs index 316de02..4682208 100644 --- a/crates/app/src/lib.rs +++ b/crates/app/src/lib.rs @@ -3,6 +3,7 @@ pub mod config; pub mod endpointing; pub mod livekit_launch; pub mod llm_config; +pub mod memory; pub mod persona; pub mod prompts; pub mod speech_bootstrap; diff --git a/crates/app/src/llm_config.rs b/crates/app/src/llm_config.rs index f61cffd..8e54fc6 100644 --- a/crates/app/src/llm_config.rs +++ b/crates/app/src/llm_config.rs @@ -53,13 +53,33 @@ impl ConfigLlmProviders { "llm.model must be set in the configuration file", )); } + // The two slots share an endpoint and a key, and differ in what is being + // asked for. Extraction is a parsing job with one right answer, so its + // sampling is fixed here rather than offered as a dial: a warmer + // extraction model invents facts about a person. + let (model_name, temperature, frequency_penalty) = match provider_key { + ProviderKey::Conversation => ( + settings.llm.model.clone(), + settings.llm.temperature, + settings.llm.frequency_penalty, + ), + ProviderKey::Assistant => { + let model = settings.memory.model.trim(); + let model = if model.is_empty() { + settings.llm.model.clone() + } else { + model.to_owned() + }; + (model, 0.0, 0.0) + } + }; Ok(LlmProviderConfig { provider_key, endpoint_url: self.endpoint.base_url.clone(), api_key: self.endpoint.api_key.clone(), - model_name: settings.llm.model.clone(), - temperature: settings.llm.temperature, - frequency_penalty: settings.llm.frequency_penalty, + model_name, + temperature, + frequency_penalty, updated_at: chrono::Utc::now(), }) } diff --git a/crates/app/src/memory.rs b/crates/app/src/memory.rs new file mode 100644 index 0000000..f44fe27 --- /dev/null +++ b/crates/app/src/memory.rs @@ -0,0 +1,54 @@ +//! Getting extraction off the turn path. +//! +//! ADR-0022 says the turn schedules and never waits. This is the only place that +//! knows how that happens — the conversation core decides *when*, the composition +//! root decides *how*. It is also where the one-at-a-time rule lives: a schedule +//! arriving while that session is already extracting is dropped, because the run +//! already going will read the same turns plus more. + +use std::collections::HashSet; +use std::sync::{Arc, Mutex}; + +use agent::{MemoryExtractionScheduler, MemoryService}; + +pub struct SpawningExtractionScheduler { + memory: Arc, + in_flight: Arc>>, +} + +impl SpawningExtractionScheduler { + pub fn new(memory: Arc) -> Self { + Self { + memory, + in_flight: Arc::new(Mutex::new(HashSet::new())), + } + } +} + +impl MemoryExtractionScheduler for SpawningExtractionScheduler { + fn schedule(&self, session_id: &str) { + { + let mut in_flight = match self.in_flight.lock() { + Ok(guard) => guard, + Err(poisoned) => poisoned.into_inner(), + }; + if !in_flight.insert(session_id.to_owned()) { + tracing::info!( + session_id, + "memory extraction already running for this session; skipped" + ); + return; + } + } + let memory = self.memory.clone(); + let in_flight = self.in_flight.clone(); + let session_id = session_id.to_owned(); + tokio::spawn(async move { + memory.extract(&session_id).await; + match in_flight.lock() { + Ok(mut guard) => guard.remove(&session_id), + Err(poisoned) => poisoned.into_inner().remove(&session_id), + }; + }); + } +} diff --git a/crates/app/src/prompts.rs b/crates/app/src/prompts.rs index 0340a43..ac55054 100644 --- a/crates/app/src/prompts.rs +++ b/crates/app/src/prompts.rs @@ -32,8 +32,7 @@ impl ConfigPromptTemplates { PromptTemplateKey::ConversationSystem2 => &prompts.character, PromptTemplateKey::ConversationSystem3 => &prompts.scene, PromptTemplateKey::ConversationWelcomeUser => &prompts.welcome, - // Only the conversation path is served; nothing else calls this. - PromptTemplateKey::AssistantSystem => return None, + PromptTemplateKey::MemoryExtraction => &prompts.memory_extraction, }; Some(text.clone()) } diff --git a/crates/app/tests/settings.rs b/crates/app/tests/settings.rs index b00ea59..2f792bb 100644 --- a/crates/app/tests/settings.rs +++ b/crates/app/tests/settings.rs @@ -126,6 +126,7 @@ fn settings_are_replaced_whole() { llm: Default::default(), prompts: Default::default(), endpointing: Default::default(), + memory: Default::default(), }; let snapshot = std::sync::Arc::new(settings); let second = snapshot.clone(); diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index cba22a6..f3163b7 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -105,6 +105,61 @@ pub struct Settings { /// When a turn starts and when it ends. #[serde(default)] pub endpointing: EndpointingSettings, + /// What the agent remembers about a caller between calls. + #[serde(default)] + pub memory: MemorySettings, +} + +/// Long-term memory (ADR-0021, ADR-0022). +/// +/// `sonari.toml.example` switches this on, because a clean clone should show +/// what the system does. The *absence* of the section leaves it off, which is a +/// different question: a configuration file written before memory existed should +/// not silently start extracting and sending notes to the model provider. +#[derive(Debug, Clone, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct MemorySettings { + #[serde(default)] + pub enabled: bool, + /// Completed turns between extractions. + #[serde(default = "default_extract_every_turns")] + pub extract_every_turns: i32, + /// The most facts one caller may have with one persona. This is what bounds + /// how long the prompt gets. + #[serde(default = "default_max_facts")] + pub max_facts: usize, + /// Per category, so a talkative week of passing news cannot evict the + /// caller's name. + #[serde(default = "default_max_facts_per_category")] + pub max_facts_per_category: usize, + /// Which model does the extracting. Empty means the conversation model. + /// A cheaper one is usually right: this is extraction, not conversation. + #[serde(default)] + pub model: String, +} + +fn default_extract_every_turns() -> i32 { + 4 +} + +fn default_max_facts() -> usize { + 40 +} + +fn default_max_facts_per_category() -> usize { + 12 +} + +impl Default for MemorySettings { + fn default() -> Self { + Self { + enabled: false, + extract_every_turns: default_extract_every_turns(), + max_facts: default_max_facts(), + max_facts_per_category: default_max_facts_per_category(), + model: String::new(), + } + } } /// Decides the boundaries of a turn from the voice activity signal. @@ -193,6 +248,10 @@ pub struct PromptTemplates { /// The opening line of a call the agent starts. #[serde(default)] pub welcome: String, + /// How the model is asked to turn a conversation into facts. Carries + /// `{{max_facts}}`, `{{max_facts_per_category}}` and `{{categories}}`. + #[serde(default)] + pub memory_extraction: String, } #[derive(Debug, Clone, Deserialize)] @@ -274,6 +333,37 @@ impl Settings { } Ok(()) } + + /// Refused at startup rather than discovered mid-call: a zero interval would + /// extract on every turn, and a zero cap would remember nothing while + /// spending a model call finding out. + fn validate_memory(&self) -> Result<()> { + if !self.memory.enabled { + return Ok(()); + } + if self.memory.extract_every_turns < 1 { + bail!("memory.extract_every_turns must be at least 1"); + } + if self.memory.max_facts < 1 { + bail!("memory.max_facts must be at least 1"); + } + if self.memory.max_facts_per_category < 1 { + bail!("memory.max_facts_per_category must be at least 1"); + } + if self.memory.max_facts_per_category > self.memory.max_facts { + bail!( + "memory.max_facts_per_category ({}) cannot exceed memory.max_facts ({})", + self.memory.max_facts_per_category, + self.memory.max_facts + ); + } + // Without it the extraction asks for nothing in particular and stores + // whatever comes back, which is worse than being switched off. + if self.prompts.memory_extraction.trim().is_empty() { + bail!("prompts.memory_extraction must be set when memory is enabled"); + } + Ok(()) + } } fn read(path: &Path) -> Result { @@ -283,6 +373,7 @@ fn read(path: &Path) -> Result { toml::from_str(&raw).with_context(|| format!("failed to parse {}", path.display()))?; settings.validate()?; settings.validate_personas()?; + settings.validate_memory()?; Ok(settings) } @@ -327,6 +418,7 @@ pub fn load_and_watch(path: &Path) -> Result { llm: LlmSettings::default(), prompts: PromptTemplates::default(), endpointing: EndpointingSettings::default(), + memory: MemorySettings::default(), } }; diff --git a/crates/platform/postgres/migrations/20260817_000001_agent_memory_facts.sql b/crates/platform/postgres/migrations/20260817_000001_agent_memory_facts.sql new file mode 100644 index 0000000..2b1a070 --- /dev/null +++ b/crates/platform/postgres/migrations/20260817_000001_agent_memory_facts.sql @@ -0,0 +1,27 @@ +-- What the agent remembers about a caller (ADR-0021). +-- +-- Rows, not a vector index: the set is injected whole and never searched, so +-- there is nothing to embed. The row is structured; the sentence is not, because +-- what is worth remembering about a person is an open set. +-- +-- Keyed on the caller and the persona together (ADR-0023): what one persona was +-- told, another does not know. +create table if not exists agent_memory_facts ( + id bigserial primary key, + user_id bigint not null, + character_id bigint not null, + category text not null, + content text not null, + -- Kept across a rewrite that keeps the fact, so "known since" survives the + -- model restating it in different words. + first_seen_at timestamptz not null, + updated_at timestamptz not null, + source_session_id text not null, + constraint agent_memory_facts_category + check (category in ('identity', 'relationship', 'preference', 'situation', 'commitment')), + -- Makes reconciliation three statements rather than a diff in Rust. + constraint agent_memory_facts_unique unique (user_id, character_id, content) +); + +create index if not exists agent_memory_facts_owner + on agent_memory_facts (user_id, character_id); diff --git a/docs/adr/0021-memory-is-extracted-facts-not-retrieval.md b/docs/adr/0021-memory-is-extracted-facts-not-retrieval.md new file mode 100644 index 0000000..9c19f9b --- /dev/null +++ b/docs/adr/0021-memory-is-extracted-facts-not-retrieval.md @@ -0,0 +1,79 @@ +# ADR-0021: Carry long-term memory as an extracted fact set, injected whole + +- **Status**: Accepted +- **Date**: 2026-08-17 +- **Tags**: `data`, `latency`, `scope` +- **Related**: ADR-0008, ADR-0022, ADR-0023 + +## Context + +A companion agent that forgets between calls fails at the thing it exists for. +Conversation history today is the last six turns of the current session, so +nothing survives hanging up. + +The obvious answer is retrieval: embed the caller's utterance, search past turns, +inject the top matches. It does not fit this system, for four reasons. + +**Latency.** Retrieval happens inside the turn — embed, search, then prompt. The +whole turn budget is two seconds (product.md). Published per-query figures for +hosted memory services are of the same order as that entire budget; the vendors +publishing them dispute each other's measurements, and none of them have been +reproduced here. Whatever the true number, it is not small relative to two +seconds, and it lands on the critical path. + +**Retrieval fires on similarity, and the facts that matter are not similar to +anything.** A caller does not say "how is my cat" to prompt the agent into +remembering there is a cat; they expect to be asked. What a companion must know +is unconditionally relevant, which is precisely what a relevance-ranked search +will not surface. + +**The retrieval unit is wrong.** Past turns are transcripts: filler, false +starts, and recognition errors. Injecting them puts recognition mistakes back +into the prompt as though they were established fact. + +**The volume does not call for it.** What is stably true about one caller is tens +of short sentences. Retrieval is a technique for context that does not fit; this +fits several times over. + +## Decision + +Store long-term memory as a bounded set of **facts**: one natural-language +sentence, one category from a closed list — `identity`, `relationship`, +`preference`, `situation`, `commitment`. Inject the whole set into the prompt as +one system message. Do not search it. + +Structure the row, not the sentence. A companion's facts are an open set — +"afraid of flying" belongs to no column anyone would have thought to add — so +typed fields would force a migration for every new kind of thing a person can +say. The category exists for eviction quotas and for grouping the injected text, +not to make the fact machine-readable. + +A model rewrites the whole set from the previous set plus recent turns, and the +result replaces it. There is no per-fact deduplication. + +## Consequences + +- The turn path gains one indexed local `SELECT` and no network call. The + two-second budget is untouched. +- What the agent knows is a finite, readable set. A test can assert on it, a + caller can be shown it, and a caller can delete it. +- Categories give eviction something to be fair about: a cap per category keeps a + talkative week of `situation` facts from evicting the caller's name. +- Cost: rewriting the whole set is lossy. The model can silently drop a fact it + should have kept. The raw turns remain in `llm_messages`, so a damaged set can + be rebuilt; the set is small enough to read; and `GET /api/memory` exists partly + so the loss is visible rather than theoretical. +- Cost: prompt length grows with the set. The cap is what bounds it, and the cap + is configuration. +- Retrieval is not ruled out — it is the right tool for episodic memory, which is + a later task. The `pgvector` image stays as it is (architecture.md §6); nothing + here uses it. + +## Alternatives considered + +| Alternative | Why not | +|---|---| +| Vector retrieval over past turns | In-turn latency against a two-second budget; fires on similarity when the needed facts are unconditional; retrieves transcripts rather than conclusions | +| Typed columns (`name`, `occupation`, `pets`) | The set of things worth remembering about a person is open; every new kind is a migration | +| One free-text block rewritten each time | Nothing to cap, nothing to evict fairly, nothing to assert on, and single facts cannot be deleted | +| Keep every past turn in the prompt | Unbounded prompt growth; recognition errors accumulate; cost per turn rises with the length of the relationship | diff --git a/docs/adr/0022-memory-extraction-runs-off-the-turn-path.md b/docs/adr/0022-memory-extraction-runs-off-the-turn-path.md new file mode 100644 index 0000000..a8d0bd9 --- /dev/null +++ b/docs/adr/0022-memory-extraction-runs-off-the-turn-path.md @@ -0,0 +1,61 @@ +# ADR-0022: Run memory extraction off the turn path + +- **Status**: Accepted +- **Date**: 2026-08-17 +- **Tags**: `latency`, `process`, `data` +- **Related**: ADR-0012, ADR-0021 + +## Context + +Turning conversation into facts costs a model call. The turn budget is two +seconds end to end (product.md), and a second model call inside a turn would +spend a large part of it on work the caller is not waiting for. + +There is a real trade in when the extraction runs. Doing it when the call ends is +cheapest, but a call that never ends cleanly — the common case, since a caller +hangs up — loses everything learned in it. Doing it during the call keeps long +calls current, at the price of concurrent work beside a live conversation. + +## Decision + +Extract every N completed turns, N being configuration, on a task spawned outside +the session task. The turn path schedules the work and returns; it never awaits +it. + +Extraction reads the recent turns and the current fact set, asks the model for a +replacement set, and writes it. Everything it touches is already persisted, so it +holds no reference to live session state. + +One extraction per session at a time. A schedule arriving while one is running is +dropped, not queued: the next one will see the same turns plus more. + +Extraction failure — an unreachable endpoint, output that will not parse — is +logged and abandoned. The stored set is left as it was. + +## Consequences + +- The turn path costs one scheduling call: a modulo and a spawn. +- A long call keeps its memory current rather than banking all of it against a + hang-up that may never be observed. +- Facts learned in the current call are not in the fact set until the next + extraction lands. Within the call this costs nothing, because the six-turn + window still carries them; across calls, a fact said in the last turns before a + hang-up can be missed. That is the accepted cost of having no reliable + end-of-call signal. +- Memory can degrade without conversation degrading, which is the rule + persistence already follows (architecture.md §3). +- Cost: a background model call runs beside a live one, adding load and spend per + call. `extract_every_turns` is what bounds it. +- Cost: the write is last-writer-wins over a whole set. Two calls by the same + caller to the same persona at once can lose one side's facts. This is not + defended against; one caller holding two simultaneous calls is not a case this + system has. + +## Alternatives considered + +| Alternative | Why not | +|---|---| +| Extract inside the turn | A second model call on the critical path against a two-second budget | +| Extract when the call ends | Callers hang up; the end is not reliably observed, and everything learned goes with it | +| Extract on a periodic sweep over all sessions | A scheduler and a claim protocol to discover what one line at the end of a turn already knows | +| Queue overlapping extractions | The queued run would read the same rows plus a few more; the queue buys nothing and can grow | diff --git a/docs/adr/0023-memory-is-scoped-to-caller-and-persona.md b/docs/adr/0023-memory-is-scoped-to-caller-and-persona.md new file mode 100644 index 0000000..d6fe221 --- /dev/null +++ b/docs/adr/0023-memory-is-scoped-to-caller-and-persona.md @@ -0,0 +1,43 @@ +# ADR-0023: Scope long-term memory to the caller and the persona + +- **Status**: Accepted +- **Date**: 2026-08-17 +- **Tags**: `data`, `scope` +- **Related**: ADR-0011, ADR-0021 + +## Context + +Facts are learned inside a conversation with one persona. Whether another persona +should see them is a product question, not a storage one. A `uid` reaches the +same history on any device (product.md), which makes the caller the obvious key; +the question is whether the persona is part of it. + +## Decision + +Key the fact set on `(user_id, character_id)`. What a caller told one persona is +not visible to another. + +## Consequences + +- Each persona's knowledge matches its own history with the caller. A persona + never refers to something it was never told, which is the failure that reads as + broken rather than merely forgetful. +- The key is already on `AgentSession`; no new identity is introduced, and + ADR-0011 still holds — this is not a tenant dimension, it is the two ids the + session already carries. +- Deletion has a natural narrow form and a natural broad one: one persona's + facts, or everything the caller has anywhere. +- Cost: a caller who switches persona starts over. For a companion that is + arguably correct, but it is a real loss and callers will notice it. +- Cost: storage multiplies by the number of personas a caller talks to. At tens + of sentences per pair this does not matter. +- Cost: the same fact is extracted once per persona, so the same model call + happens more than once across personas. + +## Alternatives considered + +| Alternative | Why not | +|---|---| +| Scope by `uid` alone | One persona speaks about things it was never told; for a companion that breaks the character more than forgetting does | +| Scope by `uid`, with per-persona visibility rules | A policy layer nobody has asked for, over a set of tens of sentences | +| Scope by session | That is the six-turn window, which already exists; it is not long-term memory | diff --git a/docs/adr/README.md b/docs/adr/README.md index 5f1916a..d97629e 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -45,6 +45,9 @@ Reading the whole directory costs roughly 400 tokens per record. Reading this in | [0018](0018-browser-test-client-inside-the-binary.md) | A browser test client is served by the binary at `/dev`, assets compiled in; Android stays the product surface | Accepted | `ops` `scope` | 2026-08-16 | | [0019](0019-vendor-the-livekit-browser-sdk.md) | The LiveKit browser SDK is vendored at a pinned version rather than fetched from a CDN or built with npm | Accepted | `ops` `audio` | 2026-08-16 | | [0020](0020-personas-are-listed-over-the-api.md) | `GET /api/personas` publishes the derived persona ids, unauthenticated, so no client recomputes them | Accepted | `scope` `ops` | 2026-08-16 | +| [0021](0021-memory-is-extracted-facts-not-retrieval.md) | Long-term memory is a bounded set of categorised natural-language facts, injected whole; nothing is searched | Accepted | `data` `latency` `scope` | 2026-08-17 | +| [0022](0022-memory-extraction-runs-off-the-turn-path.md) | Facts are extracted every N turns on a spawned task; the turn path schedules and never awaits | Accepted | `latency` `process` `data` | 2026-08-17 | +| [0023](0023-memory-is-scoped-to-caller-and-persona.md) | The fact set is keyed on `(user_id, character_id)`; a persona sees only what it was told | Accepted | `data` `scope` | 2026-08-17 | ## Superseded @@ -56,7 +59,7 @@ Reading the whole directory costs roughly 400 tokens per record. Reading this in ## By tag -`process` 0002 0003 0004 0005 0006 0013 0014 · `audio` 0003 0004 0005 0007 0009 0014 0016 0019 · `latency` 0003 0004 0009 0010 0014 0016 0017 · `providers` 0005 0006 0008 0009 0014 0015 0016 · `data` 0011 0012 · `ops` 0006 0007 0010 0012 0017 0018 0019 0020 · `scope` 0001 0002 0008 0011 0013 0015 0018 0020 +`process` 0002 0003 0004 0005 0006 0013 0014 0022 · `audio` 0003 0004 0005 0007 0009 0014 0016 0019 · `latency` 0003 0004 0009 0010 0014 0016 0017 0021 0022 · `providers` 0005 0006 0008 0009 0014 0015 0016 · `data` 0011 0012 0021 0022 0023 · `ops` 0006 0007 0010 0012 0017 0018 0019 0020 · `scope` 0001 0002 0008 0011 0013 0015 0018 0020 0021 0023 Tags are a closed vocabulary. Adding one requires a decision about what it means. diff --git a/docs/architecture.md b/docs/architecture.md index 2359402..e957e3f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -68,7 +68,7 @@ personas, start call, join the room, end call. | `sonari-config` | `sonari.toml`: parsing, validation, watching | `providers` | | `providers` | VAD on sherpa-onnx; ElevenLabs recognition and synthesis | `voice` | | `voice` | The provider traits and the runtime the call path speaks to | `shared-kernel` | -| `agent` | Prompt assembly, conversation history, the streaming model client | `shared-kernel` | +| `agent` | Prompt assembly, conversation history, long-term memory, the streaming model client | `shared-kernel` | | `call/rtc` | LiveKit rooms, tokens, track binding, PCM in/out | `shared-kernel` | | `call/speech-runtime` | Per-session speech state, rounds, endpointing policy | `voice`, `agent` | | `call/worker` | The media plane: pipeline, mixer, playback | `call/*`, `voice` | @@ -105,13 +105,24 @@ The ingress channel is bounded. Under load frames are dropped at ingress with a counter incremented, never buffered without limit. A commit is never dropped: losing a frame costs a word, losing the commit means the turn never ends. +**What the agent remembers** is assembled into the prompt with the persona: one +system message carrying the caller's fact set, then the recent turns. Reading it +is one indexed local query and no network call; a read that fails is logged and +the turn proceeds without it, because memory must never fail a call. + +Facts are written by an extraction that runs every N turns on a spawned task, +never inside a turn (ADR-0022). It reads the recent turns and the current set, +asks the model for a replacement set, and writes the difference. One runs per +session at a time; a failure leaves the stored set as it was. + --- ## 4. Configuration `sonari.toml` carries what an operator edits: personas and their scenes, the -prompts wrapped around them, which models to ask for, and the endpointing -parameters. It is watched — a change is parsed and validated, and only a valid +prompts wrapped around them, which models to ask for, the endpointing +parameters, and how memory behaves — whether it is on, how often it extracts, +and how many facts it may hold. It is watched — a change is parsed and validated, and only a valid result replaces the live one. An invalid file at startup is fatal. A session resolves its persona once at call start and holds that snapshot, so @@ -130,7 +141,7 @@ is sufficient to hold a conversation. There is no login. A `uid` is a human-typeable string; creating a session with one returns a token. `POST /api/session` and `GET /api/personas` are the two unauthenticated routes: one mints the token, the other lists what can be called -(ADR-0020). Everything under `/api/call` requires the token. The identity is derived from the `uid` rather than +(ADR-0020). Everything under `/api/call` and `/api/memory` requires the token. The identity is derived from the `uid` rather than allocated, so the same `uid` reaches the same history on any device without a user table. @@ -145,10 +156,13 @@ new layer in front, not a change to the client contract. |---|---| | `call_sessions`, `call_events`, `call_event_outbox` | One row per call; events | | `llm_sessions`, `llm_messages`, `llm_usage_logs` | Conversation history and usage | +| `agent_memory_facts` | What is remembered about a caller, per persona | | `app_error_*` | Recorded failures | -pgvector extends this schema when long-term memory lands; the image is chosen to -allow it without replacement. +Long-term memory is a set of rows, not a vector index (ADR-0021): a category and +one natural-language sentence, keyed on `(user_id, character_id)`. It is injected +whole, so nothing searches it. The pgvector image stays for episodic memory, +which is a later task; no column uses it today. --- @@ -179,6 +193,8 @@ measurements come from release builds. | Model endpoint unreachable | Turn fails | Spoken error notice; session continues | | LiveKit connection lost | Session ends | Client reconnects and starts a new session | | PostgreSQL unreachable | Facts not persisted | Calls continue; persistence never blocks audio | +| Memory read fails | The turn runs without the fact set | Logged; the agent is forgetful, not broken | +| Memory extraction fails | Nothing new is remembered | Logged; the stored set is left untouched | --- diff --git a/docs/product.md b/docs/product.md index 4486427..1c210df 100644 --- a/docs/product.md +++ b/docs/product.md @@ -26,12 +26,13 @@ exists to make that testable. | Interruption | Speaking over the agent stops it | | Personas | Operator-authored: a character and the scene they are in | | Identity | A `uid` the caller enters or is assigned | +| Memory | What the agent knows about the caller survives the call, per persona | | Client | Android — enter a `uid`, choose a character and scene, talk | | Trying it by hand | A browser test client at `/dev`, served by the binary itself (ADR-0018) | | Evaluation | An automated harness and a headless caller, both runnable in CI | -**v2** — long-term memory, and work on how human the agent sounds. v1 records -the `uid` on every session so memory has history to work with when it arrives. +**v2** — work on how human the agent sounds, and episodic memory: recalling what +happened in a particular past call, rather than what is true about the caller. **Not built**: SDK surface, billing, admin console, multi-tenancy, consumer login, tool calling, self-hosted inference. @@ -57,6 +58,22 @@ login, tool calling, self-hosted inference. edited often. Editing one takes effect on the next call without a restart. - A persona names the voice it speaks with. +### Memory + +- The agent remembers what is true about the caller — who they are, who is around + them, what they like, what is going on, what was promised — and still knows it + on the next call. +- What one persona was told, another does not know. A companion that refers to + something it was never told is more broken than one that has forgotten. +- Remembering is bounded and legible: tens of short sentences, not a transcript. + A caller can read what is held about them and delete it, all of it or one + persona's worth. +- Memory never costs the caller time. Recalling is a local lookup; nothing + between the caller finishing and the agent answering waits on a model or on + anything outside the deployment. Writing what was learned happens off that + path entirely. +- A memory failure is forgetfulness, not a failed call. + ### Identity - No account, no password, no phone number. A caller presents a `uid` — a short @@ -90,11 +107,19 @@ are synthesised by one; transcripts go to a model provider. Anyone running this for other people is sending those people's voices to third parties, and should say so. +The agent also keeps notes on the person it is talking to, written by a model +from what they said, and sends them to the model provider on every call. A `uid` +identifies but does not authenticate, so anyone who types someone's `uid` can +read those notes and can delete them. That is a consequence of having no login, +and it is stated here rather than left to be discovered. + ## 5. Out of scope, and why | | | |---|---| | Accounts and login | A companion does not need to know who you are, only which conversation is yours | +| Searching past conversations | What a companion must know is unconditional, not similar to the current sentence; retrieval belongs to episodic memory, which is v2 (ADR-0021) | +| Editing memory by hand | A caller can read and delete what is held; authoring it is a product surface nobody has asked for | | Self-hosted models | The engineering interest is the pipeline, not operating GPUs (ADR-0014) | | Tool calling | v1 is conversation. Tools add a second round trip inside a turn, which a phone call feels | | Multi-tenancy | One deployment, one operator, personas in a file | diff --git a/sonari.toml.example b/sonari.toml.example index db21ed9..63aa28e 100644 --- a/sonari.toml.example +++ b/sonari.toml.example @@ -87,6 +87,57 @@ welcome = ''' Open the call with a single short greeting, under ten words. ''' +# How a conversation becomes facts. Only used when [memory] is enabled. +# +# The reply is parsed, so the JSON shape is not decoration. Facts that do not +# parse, or that carry a category outside the list, are dropped; a reply that is +# not a fact set at all leaves what is already stored untouched. +memory_extraction = ''' +You are maintaining a short set of notes about one person, for a companion who +speaks with them on the phone. + +You are given what is known so far and the conversation since. Return the full +set of notes as it should now stand — keep what is still true, correct what has +changed, drop what has stopped being true, and add what was learned. + +Rules: +- Record only what the person stated or plainly implied. Never guess, and never + record anything the companion said about itself. +- One short sentence per fact, in the third person: "The caller has a cat called + Coal." +- Do not drop an `identity` fact unless the person contradicted it. +- At most {{max_facts}} facts in total, and at most {{max_facts_per_category}} + in any one category. Keep the ones that would matter most next time. +- Every category must be one of: {{categories}}. + +Reply with JSON and nothing else: + +{"facts": [{"category": "relationship", "content": "The caller has a cat called Coal."}]} +''' + +# What the agent remembers about a caller between calls (ADR-0021, ADR-0022). +# +# On here, because a clean clone should demonstrate what this system does. It +# costs a model call every few turns and it sends notes about the caller to the +# model provider, so anyone running this for other people should read §4 of +# docs/product.md before leaving it on. +# +# Omitting this section entirely leaves memory off: an existing configuration +# file that predates it does not silently start extracting. +[memory] +enabled = true +# Completed turns between extractions. Extraction runs off the turn path, so this +# bounds cost and background load, not latency. +extract_every_turns = 4 +# What bounds how long the prompt gets. +max_facts = 40 +# Per category, so a talkative week of `situation` facts cannot evict the +# caller's name. +max_facts_per_category = 12 +# Which model does the extracting. Empty means the conversation model above. A +# cheaper one is usually right: this is extraction, not conversation. +model = "" + # Who the agent is. At least one persona is needed before a call can start. # The id a client uses is derived from the name, so adding or reordering entries # leaves existing sessions pointing at the same persona. From 0dde357f1ea636bc7408c760881c3e8f905e9111 Mon Sep 17 00:00:00 2001 From: coderloganli Date: Tue, 18 Aug 2026 21:25:42 -0400 Subject: [PATCH 2/2] Document the memory subsystem and its decision points MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An ADR records the decision that was taken. Memory has more rejected options than taken ones — whether to retrieve or inject whole, rewrite or patch, close the category list or leave it open — and none of that was written down anywhere a person could find it. docs/memory.md carries both: what the four kinds of memory are and which exist here, how semantic memory is stored, read and written, and then thirteen decision points with the options and what each costs. The assumptions nobody has measured are listed as such, including the one that would overturn a structural decision. Linked from docs/README.md, architecture.md and the retrieval table in CLAUDE.md, so it is reachable rather than merely present. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015SH5nghQRqj6797aj4FTVk --- CLAUDE.md | 1 + docs/README.md | 4 + docs/architecture.md | 3 + docs/memory.md | 342 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 350 insertions(+) create mode 100644 docs/memory.md diff --git a/CLAUDE.md b/CLAUDE.md index dceba1b..1608220 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -9,6 +9,7 @@ Documentation is tiered so that answering a question costs a bounded amount of c | Question | Read | |---|---| | How is the system built? Where does my code go? | `docs/architecture.md` — always current, self-contained | +| What does the agent remember, and why that shape? | `docs/memory.md` — the one subsystem document; carries the options that were rejected as well as the ones taken | | What was decided about X? | `docs/adr/README.md` index only. The `Decision` column answers most questions outright | | Why was it decided that way? What was rejected? | The one or two specific ADRs the index points to | | Which decisions touch area X? | The `By tag` line in `docs/adr/README.md` | diff --git a/docs/README.md b/docs/README.md index c65625e..a13f43d 100644 --- a/docs/README.md +++ b/docs/README.md @@ -6,8 +6,12 @@ |---|---|---| | [product.md](product.md) | **What it is for.** Scope, requirements, what is deliberately absent | You are deciding whether a change belongs | | [architecture.md](architecture.md) | **How the system is built.** Components, domain model, interfaces, state machine, concurrency, failure handling | You are writing code and need to know where it goes | +| [memory.md](memory.md) | **What the agent remembers.** The kinds of memory, how semantic memory works, and every decision point with the options not taken | You are changing memory, or asking why it is shaped this way | | [adr/](adr/) | **Why it is built that way.** One decision per record, with the alternatives that were rejected | You disagree with something, or you are about to change it | +`memory.md` is the one subsystem document. It exists because memory has more +rejected options than decided ones, and an ADR records only what was chosen. + The split is deliberate. `architecture.md` describes the system as it stands and stays current. ADRs are dated snapshots of reasoning and are **never edited after acceptance** — when a decision changes, a new ADR supersedes the old one and the old one stays on disk. --- diff --git a/docs/architecture.md b/docs/architecture.md index e957e3f..ac3d058 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -164,6 +164,9 @@ one natural-language sentence, keyed on `(user_id, character_id)`. It is injecte whole, so nothing searches it. The pgvector image stays for episodic memory, which is a later task; no column uses it today. +The subsystem — the kinds of memory, how extraction and injection work, and the +options that were rejected — is [memory.md](memory.md). + --- ## 7. Observability diff --git a/docs/memory.md b/docs/memory.md new file mode 100644 index 0000000..af406aa --- /dev/null +++ b/docs/memory.md @@ -0,0 +1,342 @@ +# Sonari Memory + +What the agent remembers, how it learns it, and every choice that shaped it. + +This is the design document for one subsystem. [architecture.md](architecture.md) +says where memory sits in the system; [product.md](product.md) says what it is +for; the [ADRs](adr/README.md) hold the decisions that were made and are +immutable. **This document holds the option space** — including the options not +taken — so that anyone changing memory sees the whole board rather than only the +square that was chosen. + +--- + +# Part 1 — The memory systems + +## 1.1 Four kinds, three states + +The vocabulary is the ordinary one: what is remembered within a conversation, +what is true about a person, what happened, and how to behave. + +| Kind | What it holds | In Sonari | Status | +|---|---|---|---| +| **Working / short-term** | The current conversation | Last six turns of the session, re-read each turn | Exists, unexamined | +| **Semantic** | Facts about the caller | `agent_memory_facts` — this document | **Built** | +| **Episodic** | What happened in a particular past call | — | Not built | +| **Procedural** | How to behave | Personas and prompts in `sonari.toml` | Exists | + +"How far we got in this call" is not a fourth kind. It is working memory, and +here it is whatever fits in the six-turn window. Extending it means a rolling +summary, not a new store. + +## 1.2 What semantic memory is + +One fact is a **category** and **one sentence of natural language**, plus three +pieces of metadata: + +``` +category: relationship +content: "The caller has a cat called Coal." +first_seen_at: when it was first learned +updated_at: when an extraction last confirmed it +source_session_id: which call confirmed it +``` + +**The row is structured; the sentence is not.** What is worth remembering about a +person is an open set — "afraid of flying" belongs to no column anyone would have +thought to add — so typed fields would force a migration for every new kind of +thing a person can say (ADR-0021). + +The category exists for two mechanical purposes and no others: giving eviction a +quota to be fair about, and grouping the injected text. It does not make the fact +machine-readable. + +| Category | Holds | Note | +|---|---|---| +| `identity` | Name, age, where they live, what they do | Most stable; the prompt forbids dropping one without contradiction | +| `relationship` | Family, friends, pets, colleagues | | +| `preference` | What they like, dislike, will not discuss | | +| `situation` | What is going on now | The category that expires | +| `commitment` | What was agreed between them and the agent | Strongest effect on a companion; most often missed by general-purpose memory | + +The list order is the rendering order: stable first, passing last. + +## 1.3 Storage + +```sql +create table agent_memory_facts ( + id bigserial primary key, + user_id bigint not null, + character_id bigint not null, + category text not null, + content text not null, + first_seen_at timestamptz not null, + updated_at timestamptz not null, + source_session_id text not null, + constraint agent_memory_facts_category + check (category in ('identity','relationship','preference','situation','commitment')), + constraint agent_memory_facts_unique unique (user_id, character_id, content) +); +create index agent_memory_facts_owner on agent_memory_facts (user_id, character_id); +``` + +No vector column. The set is injected whole and never searched, so there is +nothing to embed. The `pgvector` image stays for episodic memory (architecture.md +§6); no column uses it today. + +Keyed on `(user_id, character_id)`: what one persona was told, another does not +know (ADR-0023). + +## 1.4 Reading — on the turn path + +`build_chat_messages` and `generate_welcome_message` each load the set for the +session's `(user_id, character_id)` and render it into one system message: + +``` +system: conversation_system / character / scene ← persona +system: what you already know about this person ← memory +[the recent six turns] +user: this utterance +``` + +Everything is injected; nothing is selected. The query is ordered explicitly by +`first_seen_at, id` so the text does not reshuffle between turns of one call. + +**Cost: one indexed local query, no network call.** A read that fails is logged +and the turn proceeds without the message — memory failing makes the agent +forgetful, never makes a call fail. + +Rendering an empty set produces nothing at all, so an agent with no memory sends +exactly the prompt it sent before this existed. + +## 1.5 Writing — off the turn path + +Every `extract_every_turns` completed turns, `chat_once` calls +`MemoryExtractionScheduler::schedule` and returns. The composition root's +scheduler spawns the work; the turn never awaits it (ADR-0022). + +The extraction: + +1. Loads the session, the current fact set, and the last `extract_every_turns` + turns. +2. Sends the model the **whole current set plus those turns**, and asks for the + set as it should now stand — not a list of edits (ADR-0021). +3. Parses the JSON reply. Facts with a category outside the closed list are + dropped and counted; a reply that is not a fact set at all is abandoned. +4. Validates: trims, drops empties, deduplicates on content case-insensitively, + caps per category, caps the total, in the order the model gave. +5. Replaces the stored set in one transaction — a fact whose content is unchanged + keeps its `first_seen_at`, a fact absent from the new set is deleted, a new + one is inserted. + +**A fact disappears by not being mentioned again**, which is the same action as +the model failing to mention it. That is the central cost of this shape; see +D3. + +One extraction per session at a time; a schedule arriving while one is running is +dropped. The extraction window equals the cadence, so turns covered by a dropped +extraction are not revisited by the next one. + +Two refusals protect the stored set: an unparseable reply and an empty validated +set both leave it untouched. A model having a bad turn is far likelier than a +caller whose every fact stopped being true, and the two mistakes do not cost the +same. + +## 1.6 The caller's own view + +| Route | Does | +|---|---| +| `GET /api/memory` | Every fact held for this caller, across personas | +| `DELETE /api/memory` | Forgets all of it; `?character_id=N` narrows it to one persona | + +Both behind the ordinary token; the caller is `claims.subject_id`. Nothing in the +request names whose memory it is, so nothing in the request can ask for someone +else's. + +Read and delete only. This exists because a `uid` identifies without +authenticating, so notes kept about a person have to be visible to them +(product.md §4) — and because it is the only way to see what extraction is +actually doing. + +## 1.7 Configuration + +```toml +[memory] +enabled = true +extract_every_turns = 4 # completed turns between extractions +max_facts = 40 # what bounds prompt length +max_facts_per_category = 12 +model = "" # empty means the conversation model +``` + +Validated at startup: intervals and caps at least 1, per-category cap no greater +than the total, and `prompts.memory_extraction` non-empty when enabled. An +invalid file is refused rather than half-applied. + +Omitting the section leaves memory off, so a configuration written before this +existed does not silently start extracting. + +The extraction model is the `Assistant` provider slot, at temperature 0 — +extraction is parsing, not style, and is not operator-tunable. + +## 1.8 Failure behaviour + +| Failure | Effect | +|---|---| +| Memory read fails | The turn runs without the fact set; logged | +| Extraction endpoint unreachable | Nothing new is remembered; stored set untouched | +| Reply will not parse | Same | +| Extraction yields no storable facts | Same | +| Two live sessions, same caller and persona | Last writer wins; not defended against (ADR-0022) | + +## 1.9 Where this design ends + +Whole-set injection is right for **tens** of facts. At `max_facts = 40` the +message is a few hundred tokens. Wanting hundreds of facts means retrieval, and +retrieval means a different design — that is the point at which this document is +rewritten rather than extended. + +## 1.10 Assumptions not yet measured + +Stated plainly because none of them are backed by data, and ADR-0010 forbids +inventing figures: + +| Assumption | Now | How it would be settled | +|---|---|---| +| `extract_every_turns = 4` fits real calls | Guess | Turn-count distribution over `llm_messages` | +| `max_facts = 40` is enough, and cheap enough | Guess | Profile growth curve; effect of prompt length on `llm_first_token` | +| Five categories cover what callers say | Guess | The `unknown_categories` count already in the extraction log | +| Whole-set rewrite loses little | **Unmeasured** | Offline: scripted conversations, repeated extractions, count facts that vanish without being superseded | +| The model orders by importance, so tail-cutting is safe | Unmeasured | Requires judged evaluation | +| The extraction prompt selects the right things | **Untested** | Needs an evaluation set; no test covers this today | + +The fourth row is the one that would change a structural decision. + +--- + +# Part 2 — Decision points + +Each is a real fork. The chosen option is marked **✓**; where a decision has an +ADR, the reasoning lives there and is not repeated. + +## D1 — How memory reaches the prompt (ADR-0021) + +| Option | For | Against | +|---|---|---| +| **✓ Inject the whole set** | No in-turn network call; unconditionally relevant facts are always present; testable and showable | Prompt grows with the set; hard cap on how much can be remembered | +| Vector retrieval per turn | Scales to thousands of facts | Embedding and search inside a two-second turn; fires on similarity when what matters is unconditional; retrieves transcripts, ASR errors included | +| Retrieve once per call, cache | Scales, and costs the turn nothing after the first | Still one network call at call start; needs a relevance signal before the caller has said anything | + +## D2 — When extraction runs (ADR-0022) + +| Option | For | Against | +|---|---|---| +| **✓ Every N turns, off the turn task** | Turn path pays a modulo and a spawn; long calls stay current | Facts from the last turns before a hang-up can be missed; background model call beside a live one | +| Inside the turn | Immediately available | A second model call on the critical path | +| At the end of the call | Cheapest; one call per conversation | Callers hang up; the end is not reliably observed, and everything learned goes with it | +| Periodic sweep over sessions | Decoupled entirely | A scheduler and a claim protocol to discover what the turn already knows | + +## D3 — How the set is updated (ADR-0021) + +**The decision most worth revisiting.** + +| Option | For | Against | +|---|---|---| +| **✓ Whole-set rewrite** | The model can *tidy* — merge, rephrase, reclassify — not only append; no ids leave the database; reconciliation is three statements; output is a final state with no partial application | A fact disappears by not being mentioned, so a model that forgets to write one deletes it; blast radius of one bad reply is the whole profile; output tokens grow with the set | +| Incremental `ADD` / `UPDATE` / `DELETE` | Blast radius is the rows named; output grows with new information only; deletion is explicit | Ids must be exposed to the model and validated back, hallucinated ids handled; merging two facts is harder to express; more test surface | +| Additive only, never delete or overwrite | Nothing is ever lost | The profile accumulates contradictions and stale facts; nothing bounds it | +| Temporal invalidation (mark superseded, keep history) | History survives; contradictions resolve without loss | A second dimension in the schema and in every read; more than tens of facts need | + +A cheap middle path exists and is not implemented: reject a rewrite that drops +`identity` facts or shrinks the set beyond a threshold, and log it. + +## D4 — The shape of one fact (ADR-0021) + +| Option | For | Against | +|---|---|---| +| **✓ Category + one natural-language sentence** | Open set of things worth remembering; readable by a person; injectable as-is | Not machine-queryable — "everyone with a cat" is not a query | +| Typed columns (`name`, `occupation`, `pets[]`) | Queryable; naturally bounded | Every new kind of fact is a migration; most of what a person says fits no column | +| One free-text block | Simplest to write and rewrite | Nothing to cap, nothing to evict fairly, nothing to assert on, no single fact can be deleted | + +## D5 — The category vocabulary + +| Option | For | Against | +|---|---|---| +| **✓ Closed list of five** | Eviction has a quota to be fair about; injected text groups; a check constraint enforces it | Facts that fit no category are dropped (counted, but dropped) | +| Open tags | Nothing is ever unclassifiable | No basis for a per-category quota; tags proliferate and mean nothing | +| No category at all | Simplest | Eviction can only cut by time, so a talkative week erases the caller's name | + +## D6 — Who a fact belongs to (ADR-0023) + +| Option | For | Against | +|---|---|---| +| **✓ `(caller, persona)`** | A persona never refers to something it was never told | Switching persona starts over; storage and extraction cost multiply by personas | +| Caller only | One profile, learned once, available everywhere | A persona speaks about things it was never told — for a companion, worse than forgetting | +| Caller, with per-persona visibility rules | Both | A policy layer nobody asked for, over tens of sentences | + +## D7 — What is dropped when the cap is reached + +| Option | For | Against | +|---|---|---| +| **✓ Cut from the tail of the model's own ordering** | No scoring machinery; the model states its priority by ordering | Rests on an unverified assumption that the ordering means anything | +| Recency + importance + relevance scoring | Principled, and the literature's answer | Needs an importance signal per fact and a scorer to produce it | +| Oldest first | Trivial and predictable | Deletes the caller's name, which is the oldest thing known about them | + +## D8 — An extraction that returns nothing usable + +| Option | For | Against | +|---|---|---| +| **✓ Leave the stored set untouched** | A model having a bad turn cannot erase a person | The model can never legitimately clear a profile; only `DELETE /api/memory` can | +| Apply it — an empty set means forget everything | The model's judgement is respected | One bad reply erases a relationship | + +## D9 — What the caller can do with their profile + +| Option | For | Against | +|---|---|---| +| **✓ Read and delete** | Answers the privacy question a `uid` creates; the only way to see extraction quality | A wrong fact can only be deleted, not corrected | +| Nothing — logs only | Smallest surface | What is held about a person is invisible to them | +| Full read/write/delete | Corrections possible | A write surface nobody asked for, in front of an identity that does not authenticate | + +## D10 — Whether memory is on by default + +| Option | For | Against | +|---|---|---| +| **✓ Off when the section is absent, on in the example file** | An existing deployment does not silently start sending notes about callers to the model provider; a clean clone still demonstrates the feature | Two states to reason about | +| On by default | The feature is never accidentally invisible | An upgrade changes what leaves the deployment, without anyone deciding | +| Off everywhere, including the example | Most conservative | `docker compose up` on a clean clone no longer shows what the system does | + +## D11 — Where the fact set is held during a call + +| Option | For | Against | +|---|---|---| +| **✓ Read per turn from PostgreSQL** | No cache lifetime to manage; matches how the session and recent turns are already read | One indexed local query per turn | +| Load once at call start, hold in the session | One query per call | Session-scoped state where there is none today, for a query that costs a local round trip | + +## D12 — Which model extracts + +| Option | For | Against | +|---|---|---| +| **✓ A configurable slot, defaulting to the conversation model, temperature 0** | A cheaper model can be used; extraction is parsing, so sampling is fixed rather than offered as a dial | One more configuration value | +| Always the conversation model | Nothing to configure | Pays conversation-model prices for a parsing job | +| A dedicated fine-tuned extractor | Best quality per token | Nothing to fine-tune on, and an operational burden this project rejects elsewhere | + +## D13 — How this is tested + +| Option | For | Against | +|---|---|---| +| **✓ Fakes throughout; no test judges model output** | Deterministic, runs in CI, needs no key or database | **Nothing verifies what gets remembered** — only what the system does with what the model said | +| Evaluation set with a judge | Covers the one thing fakes cannot | Judged scoring is itself uncertain; worth building when the prompt starts to iterate | +| Live-model integration tests | Real behaviour | Non-deterministic, cannot gate CI, costs money per run | + +--- + +## Related records + +- [ADR-0021](adr/0021-memory-is-extracted-facts-not-retrieval.md) — an extracted + fact set, injected whole; not retrieval +- [ADR-0022](adr/0022-memory-extraction-runs-off-the-turn-path.md) — extraction + every N turns, off the turn task +- [ADR-0023](adr/0023-memory-is-scoped-to-caller-and-persona.md) — scoped to the + caller and the persona +- [ADR-0008](adr/0008-text-core-as-first-class-entrypoint.md) — memory lives in + the text core, so the audio path does not know it exists