mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
feat(learning): privilege explicit user reflections in agent context (#1177)
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<dyn PromptSection>,
|
||||
) -> 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<String> {
|
||||
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<String> {
|
||||
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<String> {
|
||||
UserReflectionsSection.build(ctx)
|
||||
}
|
||||
|
||||
/// Render the `## Tools` catalogue in the dispatcher's tool-call format.
|
||||
pub fn render_tools(ctx: &PromptContext<'_>) -> Result<String> {
|
||||
ToolsSection.build(ctx)
|
||||
|
||||
@@ -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"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -60,6 +60,13 @@ pub struct LearnedContextData {
|
||||
pub patterns: Vec<String>,
|
||||
/// Learned user profile entries.
|
||||
pub user_profile: Vec<String>,
|
||||
/// 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<String>,
|
||||
/// 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.
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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<String>,
|
||||
#[serde(default)]
|
||||
pub user_preferences: Vec<String>,
|
||||
/// 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<String>,
|
||||
}
|
||||
|
||||
/// 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<String>,
|
||||
) -> 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<String> {
|
||||
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<String> = 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<String> {
|
||||
let mut out: Vec<String> = 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<String> = 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(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<dyn Memory> = 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<String> {
|
||||
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<dyn Memory> = 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<dyn Memory> = 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<String> = 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]
|
||||
|
||||
Reference in New Issue
Block a user