From c5ed4f5301ec8a760f5b40ee55f2292eee3d492e Mon Sep 17 00:00:00 2001 From: obchain <167975049+obchain@users.noreply.github.com> Date: Tue, 5 May 2026 23:55:07 +0530 Subject: [PATCH] feat(learning): privilege explicit user reflections in agent context (#1177) --- .../agent/harness/session/builder.rs | 16 +- src/openhuman/agent/harness/session/turn.rs | 24 +++ src/openhuman/agent/prompts/mod.rs | 78 ++++++++ src/openhuman/agent/prompts/mod_tests.rs | 128 +++++++++++++ src/openhuman/agent/prompts/types.rs | 7 + src/openhuman/learning/prompt_sections.rs | 2 + src/openhuman/learning/reflection.rs | 180 +++++++++++++++++- src/openhuman/learning/reflection_tests.rs | 171 +++++++++++++++++ 8 files changed, 602 insertions(+), 4 deletions(-) diff --git a/src/openhuman/agent/harness/session/builder.rs b/src/openhuman/agent/harness/session/builder.rs index 7cfb37bfd..5b399cc8e 100644 --- a/src/openhuman/agent/harness/session/builder.rs +++ b/src/openhuman/agent/harness/session/builder.rs @@ -704,14 +704,28 @@ impl Agent { None => SystemPromptBuilder::with_defaults(), }; if config.learning.enabled { + // Insert the privileged reflection block ahead of the + // generic `user_memory` section when one is already + // present (the `with_defaults` chain includes it). For + // builders that do not contain `user_memory` (dynamic / + // sub-agent prompts), the helper falls back to appending, + // which still keeps reflections ahead of the + // learned-context / user-profile blocks added immediately + // after. prompt_builder = prompt_builder + .insert_section_before( + "user_memory", + Box::new(crate::openhuman::context::prompt::UserReflectionsSection), + ) .add_section(Box::new( crate::openhuman::learning::LearnedContextSection::new(memory.clone()), )) .add_section(Box::new( crate::openhuman::learning::UserProfileSection::new(memory.clone()), )); - log::info!("[learning] prompt sections registered (learned_context, user_profile)"); + log::info!( + "[learning] prompt sections registered (user_reflections, learned_context, user_profile)" + ); } // Build post-turn hooks when learning is enabled diff --git a/src/openhuman/agent/harness/session/turn.rs b/src/openhuman/agent/harness/session/turn.rs index 78efe39ef..1f230c7b4 100644 --- a/src/openhuman/agent/harness/session/turn.rs +++ b/src/openhuman/agent/harness/session/turn.rs @@ -1217,6 +1217,21 @@ impl Agent { .await .unwrap_or_default(); + // Explicit user reflections — privileged memory class. Pulled + // separately from observations/patterns so the prompt assembly + // can render them ahead of generic tree summaries. + let reflection_entries = self + .memory + .list( + Some(crate::openhuman::learning::reflection::REFLECTIONS_NAMESPACE), + Some(&MemoryCategory::Custom( + crate::openhuman::learning::reflection::REFLECTIONS_NAMESPACE.into(), + )), + None, + ) + .await + .unwrap_or_default(); + // Pull every namespace's root-level summary from the tree // summarizer. This is the densest user memory we can hand the // orchestrator: each root holds up to 20 000 tokens of distilled @@ -1251,6 +1266,15 @@ impl Agent { .take(20) .map(|e| sanitize_learned_entry(&e.content)) .collect(), + // Cap reflections at 10 to keep the privileged section + // bounded — the issue requires reflections improve context + // rather than flood it. Newest first. + reflections: reflection_entries + .iter() + .rev() + .take(10) + .map(|e| sanitize_learned_entry(&e.content)) + .collect(), tree_root_summaries, } } diff --git a/src/openhuman/agent/prompts/mod.rs b/src/openhuman/agent/prompts/mod.rs index b311a3430..c295d71f2 100644 --- a/src/openhuman/agent/prompts/mod.rs +++ b/src/openhuman/agent/prompts/mod.rs @@ -35,6 +35,13 @@ impl SystemPromptBuilder { // model has rich, persistent context about the user before it // sees the tool catalogue. Section is empty (and skipped) when // the tree summarizer has nothing on disk yet. + // + // The privileged `UserReflectionsSection` is appended + // dynamically by `session::builder` when the + // learning subsystem is enabled, alongside + // `LearnedContextSection` / `UserProfileSection` — those + // three are config-gated and intentionally not part of + // the static default chain. Box::new(UserMemorySection), Box::new(ToolsSection), Box::new(SafetySection), @@ -140,6 +147,29 @@ impl SystemPromptBuilder { self } + /// Insert `section` immediately before the first existing section + /// whose [`PromptSection::name`] matches `target_name`. When no + /// matching section is present (most dynamic / sub-agent builders + /// do not include `user_memory`, for example), the new section is + /// appended at the end instead. + /// + /// Used by the session builder to guarantee that the privileged + /// reflection block ranks ahead of broader memory sections like + /// `user_memory`, even when the surrounding builder was assembled + /// via [`Self::with_defaults`] which already contains them. + pub fn insert_section_before( + mut self, + target_name: &str, + section: Box, + ) -> Self { + let position = self.sections.iter().position(|s| s.name() == target_name); + match position { + Some(idx) => self.sections.insert(idx, section), + None => self.sections.push(section), + } + self + } + /// Render every section in order into a single prompt string. /// /// The rendered bytes are intended to be **frozen for the whole @@ -238,6 +268,15 @@ pub struct WorkspaceSection; pub struct RuntimeSection; pub struct DateTimeSection; pub struct UserMemorySection; +/// Renders explicit user reflections — a privileged memory class +/// distinct from generic tree summaries. Rendered above +/// [`UserMemorySection`] so the orchestrator sees the user's own +/// intentional self-statements before any broader summary block. +/// +/// Empty (and skipped) when [`LearnedContextData::reflections`] is +/// empty — keeps the prompt clean for users who haven't yet expressed +/// any reflection-style content. +pub struct UserReflectionsSection; /// Renders the authenticated user's non-secret identity fields /// (`id` / `name` / `email`) into the system prompt — see issue #926. /// @@ -470,6 +509,39 @@ impl PromptSection for RuntimeSection { } } +impl PromptSection for UserReflectionsSection { + fn name(&self) -> &str { + "user_reflections" + } + + fn build(&self, ctx: &PromptContext<'_>) -> Result { + if ctx.learned.reflections.is_empty() { + return Ok(String::new()); + } + + let mut out = String::from("## User Reflections\n\n"); + out.push_str( + "Explicit reflections the user authored about themselves, their goals, \ + or how they want you to behave going forward. Treat these as \ + higher-priority than the broader user-memory summaries below: \ + they are recent, intentional, identity-relevant signals and \ + should steer your responses ahead of any generic historical \ + context.\n\n", + ); + for reflection in &ctx.learned.reflections { + let trimmed = reflection.trim(); + if trimmed.is_empty() { + continue; + } + out.push_str("- "); + out.push_str(trimmed); + out.push('\n'); + } + out.push('\n'); + Ok(out) + } +} + impl PromptSection for UserMemorySection { fn name(&self) -> &str { "user_memory" @@ -612,6 +684,12 @@ pub fn render_user_memory(ctx: &PromptContext<'_>) -> Result { UserMemorySection.build(ctx) } +/// Render the privileged `## User Reflections` block. Empty when the +/// learning subsystem has not captured any reflections yet. +pub fn render_user_reflections(ctx: &PromptContext<'_>) -> Result { + UserReflectionsSection.build(ctx) +} + /// Render the `## Tools` catalogue in the dispatcher's tool-call format. pub fn render_tools(ctx: &PromptContext<'_>) -> Result { ToolsSection.build(ctx) diff --git a/src/openhuman/agent/prompts/mod_tests.rs b/src/openhuman/agent/prompts/mod_tests.rs index 7e302a381..404e817b4 100644 --- a/src/openhuman/agent/prompts/mod_tests.rs +++ b/src/openhuman/agent/prompts/mod_tests.rs @@ -1198,3 +1198,131 @@ fn prompt_tool_constructors_and_user_memory_skip_empty_bodies() { assert!(!rendered.contains("### empty")); assert_eq!(default_workspace_file_content("missing"), ""); } + +fn ctx_with_learned(learned: LearnedContextData) -> PromptContext<'static> { + let prompt_tools: &'static [PromptTool<'static>] = &[]; + PromptContext { + workspace_dir: Path::new("/tmp"), + model_name: "test-model", + agent_id: "", + tools: prompt_tools, + skills: &[], + dispatcher_instructions: "", + learned, + visible_tool_names: &NO_FILTER, + tool_call_format: ToolCallFormat::PFormat, + connected_integrations: &[], + connected_identities_md: String::new(), + include_profile: false, + include_memory_md: false, + user_identity: None, + } +} + +#[test] +fn user_reflections_section_renders_bullets_with_priority_preamble() { + let ctx = ctx_with_learned(LearnedContextData { + reflections: vec![ + "Going forward I want concise replies".into(), + "I realized I prefer Rust over TypeScript".into(), + ], + ..Default::default() + }); + let rendered = UserReflectionsSection.build(&ctx).unwrap(); + assert!(rendered.starts_with("## User Reflections\n\n")); + assert!( + rendered.contains("higher-priority"), + "preamble must signal that reflections outrank generic memory" + ); + assert!(rendered.contains("- Going forward I want concise replies")); + assert!(rendered.contains("- I realized I prefer Rust over TypeScript")); +} + +#[test] +fn user_reflections_section_returns_empty_without_entries() { + let ctx = ctx_with_learned(LearnedContextData::default()); + assert!(UserReflectionsSection.build(&ctx).unwrap().is_empty()); +} + +#[test] +fn user_reflections_section_skips_blank_entries() { + let ctx = ctx_with_learned(LearnedContextData { + reflections: vec![" ".into(), "Real reflection".into(), "".into()], + ..Default::default() + }); + let rendered = UserReflectionsSection.build(&ctx).unwrap(); + assert!(rendered.contains("- Real reflection")); + // Bullet count should match the non-blank entry count. + assert_eq!(rendered.matches("\n- ").count(), 1); +} + +#[test] +fn render_user_reflections_helper_matches_section_output() { + let ctx = ctx_with_learned(LearnedContextData { + reflections: vec!["x".into()], + ..Default::default() + }); + let via_section = UserReflectionsSection.build(&ctx).unwrap(); + let via_helper = render_user_reflections(&ctx).unwrap(); + assert_eq!(via_section, via_helper); +} + +#[test] +fn insert_section_before_places_section_ahead_of_named_target() { + // Reflections must rank ahead of generic memory in builders that + // already include `UserMemorySection` (the `with_defaults` chain). + // Verify the helper inserts at the correct index instead of + // tail-appending. + let builder = SystemPromptBuilder::with_defaults() + .insert_section_before("user_memory", Box::new(UserReflectionsSection)); + let names: Vec<&str> = builder.sections.iter().map(|s| s.name()).collect(); + let r_idx = names + .iter() + .position(|n| *n == "user_reflections") + .expect("user_reflections section"); + let m_idx = names + .iter() + .position(|n| *n == "user_memory") + .expect("user_memory section"); + assert!( + r_idx < m_idx, + "insert_section_before should place the new section ahead of its target, got order {names:?}" + ); +} + +#[test] +fn insert_section_before_falls_back_to_append_when_target_missing() { + // Dynamic / sub-agent builders do not include a `user_memory` + // section. The helper should still land the new section so the + // caller's wiring stays loop-free, just at the tail. + let builder = SystemPromptBuilder::default() + .add_section(Box::new(SafetySection)) + .insert_section_before("user_memory", Box::new(UserReflectionsSection)); + let names: Vec<&str> = builder.sections.iter().map(|s| s.name()).collect(); + assert_eq!(names.last(), Some(&"user_reflections")); + assert_eq!(names.len(), 2); +} + +#[test] +fn user_reflections_render_above_user_memory_when_both_present() { + // Acceptance criterion: reflections rank above generic + // tree summaries — verify by composing the same way the runtime + // does (UserReflectionsSection appended ahead of any + // UserMemorySection content). + let ctx = ctx_with_learned(LearnedContextData { + reflections: vec!["I want terse answers".into()], + tree_root_summaries: vec![("user".into(), "Generic summary".into())], + ..Default::default() + }); + let reflections = UserReflectionsSection.build(&ctx).unwrap(); + let memory = UserMemorySection.build(&ctx).unwrap(); + let combined = format!("{reflections}{memory}"); + let r_idx = combined + .find("## User Reflections") + .expect("reflections heading"); + let m_idx = combined.find("## User Memory").expect("memory heading"); + assert!( + r_idx < m_idx, + "reflections must render before user-memory block" + ); +} diff --git a/src/openhuman/agent/prompts/types.rs b/src/openhuman/agent/prompts/types.rs index a83828ce6..9b0168de1 100644 --- a/src/openhuman/agent/prompts/types.rs +++ b/src/openhuman/agent/prompts/types.rs @@ -60,6 +60,13 @@ pub struct LearnedContextData { pub patterns: Vec, /// Learned user profile entries. pub user_profile: Vec, + /// Explicit user reflections captured from chat — distinct, high-priority + /// memory class. These are the user's own intentional self-statements + /// ("remember that I…", "going forward…", "I realized…") and are + /// privileged above generic [`Self::tree_root_summaries`] when the + /// orchestrator assembles its system prompt. Empty when the learning + /// subsystem is off or no reflections have been captured yet. + pub reflections: Vec, /// Pre-fetched root-level summaries from the tree summarizer, one per /// namespace that has a root node on disk. Each entry is /// `(namespace, body)`. Empty when the tree summarizer hasn't run. diff --git a/src/openhuman/learning/prompt_sections.rs b/src/openhuman/learning/prompt_sections.rs index e6b200de1..8402662f2 100644 --- a/src/openhuman/learning/prompt_sections.rs +++ b/src/openhuman/learning/prompt_sections.rs @@ -181,6 +181,7 @@ mod tests { observations: vec!["Tool use succeeded".into()], patterns: vec!["User prefers terse replies".into()], user_profile: Vec::new(), + reflections: Vec::new(), tree_root_summaries: Vec::new(), })) .unwrap(); @@ -213,6 +214,7 @@ mod tests { "Timezone: America/Los_Angeles".into(), "Prefers Rust".into(), ], + reflections: Vec::new(), tree_root_summaries: Vec::new(), })) .unwrap(); diff --git a/src/openhuman/learning/reflection.rs b/src/openhuman/learning/reflection.rs index 93d3b0ed4..2099fe7af 100644 --- a/src/openhuman/learning/reflection.rs +++ b/src/openhuman/learning/reflection.rs @@ -13,6 +13,13 @@ use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::sync::Arc; +/// Memory namespace + custom-category tag for explicit user reflections. +/// +/// Distinct from `learning_observations` (agent-extracted) and +/// `user_profile` (preference facts) — these are sentences the user +/// authored about themselves that should steer future agent behaviour. +pub const REFLECTIONS_NAMESPACE: &str = "learning_reflections"; + /// Structured output expected from the reflection LLM call. #[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct ReflectionOutput { @@ -22,6 +29,12 @@ pub struct ReflectionOutput { pub patterns: Vec, #[serde(default)] pub user_preferences: Vec, + /// Explicit user reflections lifted out of the conversation — the + /// user's own intentional self-statements ("I realized…", "going + /// forward…", "remember that I…"). Stored as a distinct memory + /// class and rendered in the prompt above generic tree summaries. + #[serde(default)] + pub user_reflections: Vec, } /// Post-turn hook that reflects on completed turns and stores observations. @@ -98,7 +111,12 @@ impl ReflectionHook { Return a JSON object with these fields:\n\ - \"observations\": array of strings — what worked, what failed, notable patterns\n\ - \"patterns\": array of strings — recurring patterns worth remembering\n\ - - \"user_preferences\": array of strings — any user preferences detected\n\n\ + - \"user_preferences\": array of strings — any user preferences detected\n\ + - \"user_reflections\": array of strings — explicit reflections the user \ + made about themselves, their goals, what they want to do differently, \ + or what they want you to remember going forward. Only include statements \ + the user clearly authored as a reflection (\"I realized…\", \"remember that I…\", \ + \"going forward I want…\"). Leave empty if none.\n\n\ Keep each entry concise (one sentence). Return ONLY valid JSON, no markdown.\n\n", ); @@ -174,6 +192,7 @@ impl ReflectionHook { observations: vec![trimmed.to_string()], patterns: Vec::new(), user_preferences: Vec::new(), + user_reflections: Vec::new(), } }) } @@ -230,8 +249,130 @@ impl ReflectionHook { .await?; } + // Reflection persistence is handled by the caller + // (`on_turn_complete`) so the heuristic fast-path and the LLM + // path share a single per-turn dedupe set and never write the + // same sentence twice. Ok(()) } + + /// Persist a single reflection sentence into the dedicated namespace. + /// Public to the crate so the heuristic fast-path can reuse the same + /// storage shape without going through the LLM round-trip. + pub(crate) async fn persist_reflection(&self, reflection: &str) -> anyhow::Result<()> { + let trimmed = reflection.trim(); + if trimmed.is_empty() { + return Ok(()); + } + let date = chrono::Local::now().format("%Y-%m-%d").to_string(); + let hash = &uuid::Uuid::new_v4().to_string()[..8]; + let key = format!("ref/{date}/{hash}"); + self.memory + .store( + REFLECTIONS_NAMESPACE, + &key, + trimmed, + MemoryCategory::Custom(REFLECTIONS_NAMESPACE.into()), + None, + ) + .await?; + log::debug!( + "[learning] stored user reflection at {key} ({} chars)", + trimmed.chars().count() + ); + Ok(()) + } + + /// Persist a reflection sentence iff its normalised form has not + /// already been seen in the current turn. `seen` is the per-turn + /// dedupe set shared between the heuristic fast-path and the LLM + /// `user_reflections` path, so a sentence captured by both routes + /// only lands in memory once. + async fn persist_reflection_deduped( + &self, + reflection: &str, + seen: &mut std::collections::HashSet, + ) -> anyhow::Result<()> { + let normalised = normalise_reflection(reflection); + if normalised.is_empty() { + return Ok(()); + } + if !seen.insert(normalised) { + log::debug!( + "[learning] reflection already captured this turn — skipping duplicate write" + ); + return Ok(()); + } + self.persist_reflection(reflection).await + } +} + +/// Normalise a reflection sentence for per-turn dedupe comparisons: +/// trim outer whitespace and lower-case so casing or trailing +/// punctuation differences do not bypass the duplicate check. +fn normalise_reflection(s: &str) -> String { + s.trim().to_ascii_lowercase() +} + +/// Heuristic detector for explicit reflection cues in a user message. +/// +/// Returns the trimmed sentences from `user_message` that match a known +/// reflection cue ("I realized", "going forward", "remember that I", +/// "I learned", "I want to", "I've decided"). Used as a fast-path so +/// reflections get captured even when the post-turn LLM reflection is +/// throttled, disabled, or routed to a slow cloud model. +/// +/// The detector is intentionally conservative — false positives would +/// flood the privileged reflection namespace and dilute its signal. +pub fn extract_reflection_cues(user_message: &str) -> Vec { + const CUES: &[&str] = &[ + "i realized", + "i realised", + "i learned", + "i learnt", + "i've decided", + "i have decided", + "going forward", + "from now on", + "remember that i", + "remember i", + "please remember", + "i want you to remember", + ]; + + let mut hits: Vec = Vec::new(); + for sentence in split_sentences(user_message) { + let lower = sentence.to_ascii_lowercase(); + if CUES.iter().any(|cue| lower.contains(cue)) { + let trimmed = sentence.trim(); + if !trimmed.is_empty() && !hits.iter().any(|h| h == trimmed) { + hits.push(trimmed.to_string()); + } + } + } + hits +} + +/// Split free text into sentence-shaped chunks on `.`, `!`, `?`, and +/// newlines. Cheap and good enough for cue detection — full NLP is +/// overkill for matching a known short cue list. +fn split_sentences(text: &str) -> Vec { + let mut out: Vec = Vec::new(); + let mut buf = String::new(); + for ch in text.chars() { + if matches!(ch, '.' | '!' | '?' | '\n') { + if !buf.trim().is_empty() { + out.push(buf.trim().to_string()); + } + buf.clear(); + } else { + buf.push(ch); + } + } + if !buf.trim().is_empty() { + out.push(buf.trim().to_string()); + } + out } #[async_trait] @@ -241,6 +382,26 @@ impl PostTurnHook for ReflectionHook { } async fn on_turn_complete(&self, ctx: &TurnContext) -> anyhow::Result<()> { + // Per-turn dedupe set: shared between the heuristic fast-path + // below and the LLM `user_reflections` persistence below, so + // the same sentence captured by both routes only lands in + // memory once and cannot crowd out unique reflections in the + // bounded top-N retrieval window. + let mut seen: std::collections::HashSet = std::collections::HashSet::new(); + + // Fast-path heuristic capture — runs whenever the learning + // subsystem is on, regardless of turn complexity, so single-turn + // reflections like "remember that I prefer terse answers" are + // promoted to the privileged reflection namespace without paying + // for a reflection-LLM round-trip. + if self.config.enabled { + for cue in extract_reflection_cues(&ctx.user_message) { + if let Err(e) = self.persist_reflection_deduped(&cue, &mut seen).await { + log::warn!("[learning] failed to persist heuristic reflection: {e}"); + } + } + } + if !self.should_reflect(ctx) { return Ok(()); } @@ -267,10 +428,11 @@ impl PostTurnHook for ReflectionHook { let output = Self::parse_reflection(&raw); log::info!( - "[learning] reflection complete: observations={} patterns={} prefs={}", + "[learning] reflection complete: observations={} patterns={} prefs={} user_reflections={}", output.observations.len(), output.patterns.len(), - output.user_preferences.len() + output.user_preferences.len(), + output.user_reflections.len(), ); if let Err(e) = self.store_reflection(&output).await { @@ -278,6 +440,18 @@ impl PostTurnHook for ReflectionHook { return Err(e); } + // Persist LLM-extracted reflections through the shared dedupe + // set so any sentence the heuristic already captured above is + // not written twice. Failures here are logged but never roll + // back the session counter — observations / patterns / + // preferences from the same turn have already been committed + // and the throttle quota is correctly accounted for. + for reflection in &output.user_reflections { + if let Err(e) = self.persist_reflection_deduped(reflection, &mut seen).await { + log::warn!("[learning] failed to persist LLM-extracted reflection: {e}"); + } + } + Ok(()) } } diff --git a/src/openhuman/learning/reflection_tests.rs b/src/openhuman/learning/reflection_tests.rs index 6ff83ad96..8d64255a7 100644 --- a/src/openhuman/learning/reflection_tests.rs +++ b/src/openhuman/learning/reflection_tests.rs @@ -236,6 +236,14 @@ async fn store_reflection_persists_all_categories() { observations: vec!["Observed failure".into()], patterns: vec!["Pattern A".into()], user_preferences: vec!["Pref A".into()], + // user_reflections are intentionally persisted by + // `on_turn_complete` (not `store_reflection`) so they share a + // per-turn dedupe set with the heuristic fast-path. This test + // therefore only asserts the observation / pattern / preference + // contracts owned by `store_reflection`; the reflection + // persistence contract is covered by the dedupe + heuristic + // tests below. + user_reflections: vec!["should not be written by store_reflection".into()], }) .await .unwrap(); @@ -244,6 +252,169 @@ async fn store_reflection_persists_all_categories() { assert!(keys.iter().any(|key| key.starts_with("obs/"))); assert!(keys.iter().any(|key| key == "pat/pattern_a")); assert!(keys.iter().any(|key| key == "pref/pref_a")); + assert!( + !keys.iter().any(|key| key.starts_with("ref/")), + "store_reflection must not persist user_reflections — that path now lives in on_turn_complete so the dedupe set is shared with the heuristic" + ); +} + +#[tokio::test] +async fn persist_reflection_writes_to_dedicated_namespace_and_category() { + let memory_impl = Arc::new(MockMemory::default()); + let memory: Arc = memory_impl.clone(); + let hook = ReflectionHook::new( + reflection_config(), + Arc::new(Config::default()), + memory, + None, + ); + + hook.persist_reflection("I want shorter answers going forward") + .await + .unwrap(); + + let entries = memory_impl.entries.lock(); + let reflection = entries + .values() + .find(|e| e.key.starts_with("ref/")) + .expect("reflection entry"); + assert_eq!(reflection.namespace.as_deref(), Some(REFLECTIONS_NAMESPACE)); + assert!(matches!( + reflection.category, + MemoryCategory::Custom(ref tag) if tag == REFLECTIONS_NAMESPACE + )); + assert_eq!(reflection.content, "I want shorter answers going forward"); +} + +#[tokio::test] +async fn on_turn_complete_dedupes_reflections_across_heuristic_and_llm_paths() { + use crate::openhuman::providers::Provider; + use async_trait::async_trait; + + // Stub provider returning a reflection LLM response whose + // `user_reflections` array repeats the same sentence the heuristic + // would also lift out of the user message. Only `chat_with_system` + // needs implementing — `simple_chat` (the call-site used by + // `ReflectionHook::run_reflection` for the cloud path) has a + // default trait impl that delegates here. + struct StubProvider; + #[async_trait] + impl Provider for StubProvider { + async fn chat_with_system( + &self, + _system_prompt: Option<&str>, + _message: &str, + _model: &str, + _temperature: f64, + ) -> anyhow::Result { + Ok(r#"{"observations":[],"patterns":[],"user_preferences":[], + "user_reflections":["Going forward I want concise replies"]}"# + .into()) + } + } + + let memory_impl = Arc::new(MockMemory::default()); + let memory: Arc = memory_impl.clone(); + let hook = ReflectionHook::new( + reflection_config(), + Arc::new(Config::default()), + memory, + Some(Arc::new(StubProvider)), + ); + + let turn = TurnContext { + // Heuristic captures this sentence; the stub LLM also returns + // the same sentence in `user_reflections`. Without per-turn + // dedupe both paths would write it. + user_message: "Going forward I want concise replies.".into(), + assistant_response: "noted".repeat(120), + tool_calls: Vec::new(), + turn_duration_ms: 50, + session_id: Some("dedupe".into()), + iteration_count: 1, + }; + hook.on_turn_complete(&turn).await.unwrap(); + + let ref_count = memory_impl + .entries + .lock() + .values() + .filter(|e| e.key.starts_with("ref/")) + .count(); + assert_eq!( + ref_count, 1, + "reflection captured by both heuristic and LLM paths must be persisted exactly once" + ); +} + +#[test] +fn parse_reflection_extracts_user_reflections_field() { + let raw = r#"{"observations":[],"patterns":[],"user_preferences":[], + "user_reflections":["I realized I want to focus on Rust this quarter"]}"#; + let output = ReflectionHook::parse_reflection(raw); + assert_eq!( + output.user_reflections, + vec!["I realized I want to focus on Rust this quarter"] + ); +} + +#[test] +fn parse_reflection_defaults_user_reflections_when_absent() { + let raw = r#"{"observations":["x"],"patterns":[],"user_preferences":[]}"#; + let output = ReflectionHook::parse_reflection(raw); + assert!(output.user_reflections.is_empty()); +} + +#[test] +fn extract_reflection_cues_picks_up_explicit_self_statements() { + let msg = "I realized I prefer terse answers. Going forward, please skip the disclaimers."; + let cues = extract_reflection_cues(msg); + assert_eq!(cues.len(), 2); + assert!(cues[0].to_ascii_lowercase().contains("i realized")); + assert!(cues[1].to_ascii_lowercase().contains("going forward")); +} + +#[test] +fn extract_reflection_cues_ignores_messages_without_cues() { + let msg = "What is the weather today? Also, can you summarise this PR?"; + assert!(extract_reflection_cues(msg).is_empty()); +} + +#[test] +fn extract_reflection_cues_dedupes_identical_sentences() { + let msg = "Remember that I work in PST. Remember that I work in PST."; + let cues = extract_reflection_cues(msg); + assert_eq!(cues.len(), 1); +} + +#[tokio::test] +async fn on_turn_complete_persists_heuristic_reflection_even_when_complexity_low() { + let memory_impl = Arc::new(MockMemory::default()); + let memory: Arc = memory_impl.clone(); + // Pin the source to local + threshold high so the LLM path is + // skipped and we observe ONLY the heuristic capture. + let mut cfg = reflection_config(); + cfg.min_turn_complexity = 99; + cfg.reflection_source = ReflectionSource::Local; + let hook = ReflectionHook::new(cfg, Arc::new(Config::default()), memory, None); + + let turn = TurnContext { + user_message: "Going forward I want concise replies only.".into(), + assistant_response: "ok".into(), + tool_calls: Vec::new(), + turn_duration_ms: 10, + session_id: Some("s".into()), + iteration_count: 1, + }; + // The LLM path is gated off by complexity, so the call returns Ok + // even without a provider — only the heuristic should write. + hook.on_turn_complete(&turn).await.unwrap(); + + let keys: Vec = memory_impl.entries.lock().keys().cloned().collect(); + assert!( + keys.iter().any(|k| k.starts_with("ref/")), + "heuristic capture should persist a reflection without LLM round-trip" + ); } #[tokio::test]