mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(prompts): frame MEMORY.md as background so fresh threads don't fake prior-chat continuity (#4748)
This commit is contained in:
@@ -933,6 +933,100 @@ fn render_subagent_system_prompt_skips_profile_md_when_include_profile_false() {
|
||||
let _ = std::fs::remove_dir_all(workspace);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_subagent_system_prompt_frames_memory_md_as_background() {
|
||||
// GH-4745 regression for the sub-agent path: Inline/File sub-agents inject
|
||||
// MEMORY.md through `render_subagent_system_prompt`, a separate renderer
|
||||
// from `UserFilesSection`. It must share the same background-memory frame,
|
||||
// otherwise a fresh thread reads the bare `### MEMORY.md` block as prior
|
||||
// in-thread conversation and asserts continuity that isn't there.
|
||||
let workspace = std::env::temp_dir().join(format!(
|
||||
"openhuman_subagent_memory_framing_{}",
|
||||
uuid::Uuid::new_v4()
|
||||
));
|
||||
std::fs::create_dir_all(&workspace).unwrap();
|
||||
std::fs::write(
|
||||
workspace.join("MEMORY.md"),
|
||||
"# Long-term memory\nReviewed `def f(x)` last week; user prefers terse notes.",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let tools: Vec<Box<dyn Tool>> = vec![Box::new(TestTool)];
|
||||
let rendered = render_subagent_system_prompt(
|
||||
&workspace,
|
||||
"test-model",
|
||||
&[0],
|
||||
&tools,
|
||||
&[],
|
||||
"You are a specialist agent.",
|
||||
SubagentRenderOptions {
|
||||
include_identity: false,
|
||||
include_safety_preamble: false,
|
||||
include_skills_catalog: false,
|
||||
include_profile: false,
|
||||
include_memory_md: true,
|
||||
},
|
||||
ToolCallFormat::PFormat,
|
||||
&[],
|
||||
);
|
||||
|
||||
assert!(
|
||||
rendered.contains("### MEMORY.md") && rendered.contains("terse notes"),
|
||||
"MEMORY.md must still be injected in the sub-agent path, got:\n{rendered}"
|
||||
);
|
||||
assert!(
|
||||
rendered.contains("background — not this conversation"),
|
||||
"sub-agent MEMORY.md must be framed as durable background memory, got:\n{rendered}"
|
||||
);
|
||||
let frame_at = rendered.find("background — not this conversation").unwrap();
|
||||
let heading_at = rendered.find("### MEMORY.md").unwrap();
|
||||
assert!(
|
||||
frame_at < heading_at,
|
||||
"the guardrail note must precede the MEMORY.md block, got:\n{rendered}"
|
||||
);
|
||||
|
||||
let _ = std::fs::remove_dir_all(workspace);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_subagent_system_prompt_omits_memory_framing_when_no_memory_content() {
|
||||
// Companion to the framing test: with `include_memory_md = true` but no
|
||||
// MEMORY.md on disk (a genuinely fresh workspace) the dangling frame must
|
||||
// NOT appear — emitting a "background memory" note pointing at nothing
|
||||
// would itself imply phantom history.
|
||||
let workspace = std::env::temp_dir().join(format!(
|
||||
"openhuman_subagent_memory_noframe_{}",
|
||||
uuid::Uuid::new_v4()
|
||||
));
|
||||
std::fs::create_dir_all(&workspace).unwrap();
|
||||
|
||||
let tools: Vec<Box<dyn Tool>> = vec![Box::new(TestTool)];
|
||||
let rendered = render_subagent_system_prompt(
|
||||
&workspace,
|
||||
"test-model",
|
||||
&[0],
|
||||
&tools,
|
||||
&[],
|
||||
"You are a specialist agent.",
|
||||
SubagentRenderOptions {
|
||||
include_identity: false,
|
||||
include_safety_preamble: false,
|
||||
include_skills_catalog: false,
|
||||
include_profile: false,
|
||||
include_memory_md: true,
|
||||
},
|
||||
ToolCallFormat::PFormat,
|
||||
&[],
|
||||
);
|
||||
|
||||
assert!(
|
||||
!rendered.contains("background — not this conversation"),
|
||||
"no MEMORY.md content → no dangling framing note in sub-agent path, got:\n{rendered}"
|
||||
);
|
||||
|
||||
let _ = std::fs::remove_dir_all(workspace);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_subagent_system_prompt_injects_profile_md_when_identity_included() {
|
||||
// When identity is on, PROFILE.md must still be injected alongside
|
||||
@@ -1407,6 +1501,101 @@ fn for_subagent_builder_injects_user_files_even_when_identity_omitted() {
|
||||
let _ = std::fs::remove_dir_all(workspace);
|
||||
}
|
||||
|
||||
/// Shared `PromptContext` for the MEMORY.md-framing tests below. Both
|
||||
/// exercise `UserFilesSection` with memory injection enabled and differ
|
||||
/// only in workspace contents, so they build an identical 19-field
|
||||
/// context — factor it out so the two can't drift when `PromptContext`
|
||||
/// gains fields. Borrows the caller's `workspace` and pre-built
|
||||
/// `prompt_tools` so the returned context outlives neither.
|
||||
fn memory_framing_ctx<'a>(
|
||||
workspace: &'a std::path::Path,
|
||||
prompt_tools: &'a [PromptTool<'a>],
|
||||
) -> PromptContext<'a> {
|
||||
PromptContext {
|
||||
workspace_dir: workspace,
|
||||
model_name: "test-model",
|
||||
agent_id: "",
|
||||
tools: prompt_tools,
|
||||
workflows: &[],
|
||||
dispatcher_instructions: "",
|
||||
learned: LearnedContextData::default(),
|
||||
visible_tool_names: &NO_FILTER,
|
||||
tool_call_format: ToolCallFormat::PFormat,
|
||||
connected_integrations: &[],
|
||||
connected_identities_md: String::new(),
|
||||
include_profile: false,
|
||||
include_memory_md: true,
|
||||
curated_snapshot: None,
|
||||
user_identity: None,
|
||||
personality_soul_md: None,
|
||||
personality_memory_md: None,
|
||||
personality_roster: vec![],
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn memory_md_injection_is_framed_as_background_not_prior_chat() {
|
||||
// GH-4745 regression: MEMORY.md is durable cross-session memory. Without
|
||||
// a frame, a relevant curated observation reads to the model as prior
|
||||
// *in-thread* conversation, so on a brand-new thread it opens with
|
||||
// "already covered this in a previous chat" and shortcuts its answer.
|
||||
// Pin that the rendered prompt frames the block as background memory and
|
||||
// that the guardrail precedes the injected `### MEMORY.md` heading.
|
||||
//
|
||||
// `tempfile::tempdir()` cleans up via `Drop` even when an assertion
|
||||
// below panics — a bare `remove_dir_all` at the tail would leak the
|
||||
// dir exactly on the failing run we most want to inspect.
|
||||
let workspace = tempfile::tempdir().unwrap();
|
||||
std::fs::write(
|
||||
workspace.path().join("MEMORY.md"),
|
||||
"# Long-term memory\nReviewed `def f(x)` last week; user prefers terse notes.",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let tools: Vec<Box<dyn Tool>> = vec![];
|
||||
let prompt_tools = PromptTool::from_tools(&tools);
|
||||
let ctx = memory_framing_ctx(workspace.path(), &prompt_tools);
|
||||
|
||||
let rendered = UserFilesSection.build(&ctx).unwrap();
|
||||
|
||||
assert!(
|
||||
rendered.contains("### MEMORY.md") && rendered.contains("terse notes"),
|
||||
"MEMORY.md must still be injected, got:\n{rendered}"
|
||||
);
|
||||
assert!(
|
||||
rendered.contains("background — not this conversation"),
|
||||
"MEMORY.md must be framed as durable background memory, got:\n{rendered}"
|
||||
);
|
||||
assert!(
|
||||
rendered.contains("already covered this in a previous chat"),
|
||||
"framing must explicitly forbid asserting prior-chat continuity, got:\n{rendered}"
|
||||
);
|
||||
let frame_at = rendered.find("background — not this conversation").unwrap();
|
||||
let heading_at = rendered.find("### MEMORY.md").unwrap();
|
||||
assert!(
|
||||
frame_at < heading_at,
|
||||
"the guardrail note must precede the MEMORY.md block, got:\n{rendered}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn memory_md_framing_absent_when_no_memory_content() {
|
||||
// The frame must never appear on its own: when MEMORY.md is missing/empty
|
||||
// (a genuinely fresh workspace) there is nothing to scope, so emitting a
|
||||
// dangling "background memory" note would itself imply phantom history.
|
||||
let workspace = tempfile::tempdir().unwrap();
|
||||
|
||||
let tools: Vec<Box<dyn Tool>> = vec![];
|
||||
let prompt_tools = PromptTool::from_tools(&tools);
|
||||
let ctx = memory_framing_ctx(workspace.path(), &prompt_tools);
|
||||
|
||||
let rendered = UserFilesSection.build(&ctx).unwrap();
|
||||
assert!(
|
||||
!rendered.contains("background — not this conversation"),
|
||||
"no MEMORY.md content → no dangling framing note, got:\n{rendered}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sync_workspace_file_updates_hash_and_inject_workspace_file_truncates() {
|
||||
let workspace = std::env::temp_dir().join(format!(
|
||||
|
||||
@@ -301,7 +301,19 @@ pub fn render_subagent_system_prompt_with_format(
|
||||
inject_workspace_file_capped(&mut out, workspace_dir, "PROFILE.md", USER_FILE_MAX_CHARS);
|
||||
}
|
||||
if options.include_memory_md {
|
||||
inject_workspace_file_capped(&mut out, workspace_dir, "MEMORY.md", USER_FILE_MAX_CHARS);
|
||||
// Frame MEMORY.md as durable, cross-session background memory —
|
||||
// byte-identical to `UserFilesSection::build` (GH-4745). Without the
|
||||
// frame an Inline/File sub-agent on a brand-new thread reads the bare
|
||||
// `### MEMORY.md` block as prior in-thread conversation and asserts
|
||||
// continuity that isn't there. Buffer first so the note is emitted
|
||||
// only when the file actually carries content — a dangling frame
|
||||
// pointing at nothing would itself imply phantom history.
|
||||
let mut mem = String::new();
|
||||
inject_workspace_file_capped(&mut mem, workspace_dir, "MEMORY.md", USER_FILE_MAX_CHARS);
|
||||
if !mem.trim().is_empty() {
|
||||
out.push_str(MEMORY_MD_FRAMING);
|
||||
out.push_str(&mem);
|
||||
}
|
||||
}
|
||||
|
||||
// 2. Filtered tool catalogue. Indices are taken in ascending order
|
||||
|
||||
@@ -185,6 +185,31 @@ pub struct UserIdentitySection;
|
||||
/// [`AgentDefinition::omit_profile`] / `omit_memory_md`.
|
||||
pub struct UserFilesSection;
|
||||
|
||||
/// Framing preamble emitted immediately before the injected `MEMORY.md`
|
||||
/// (and, in the snapshot path, `USER.md`) block.
|
||||
///
|
||||
/// `MEMORY.md` is durable, cross-session memory — archivist-curated facts
|
||||
/// carried over from *past* sessions and previously ingested history. Without
|
||||
/// a frame, a relevant curated observation reads to the model as something
|
||||
/// already said *in this thread*, so on a brand-new thread it asserts
|
||||
/// continuity that isn't there ("already covered this in a previous chat")
|
||||
/// and shortcuts its answer (GH-4745). This note scopes the block as
|
||||
/// background knowledge and forbids claiming in-thread continuity.
|
||||
///
|
||||
/// `pub(crate)` so the sub-agent renderer
|
||||
/// ([`super::render_helpers::render_subagent_system_prompt_with_format`])
|
||||
/// can share the exact same frame — Inline/File sub-agents inject
|
||||
/// `MEMORY.md` through their own path and must not drift from this note.
|
||||
pub(crate) const MEMORY_MD_FRAMING: &str =
|
||||
"### Long-term memory (background — not this conversation)\n\n\
|
||||
The block below is your durable, cross-session memory: facts and observations \
|
||||
carried over from *past* sessions and previously ingested history. Treat it as \
|
||||
background knowledge only — it is **not** part of the current thread. Do not \
|
||||
treat it as messages already exchanged here, never claim you \"already covered \
|
||||
this in a previous chat\" or otherwise assert continuity that isn't present in \
|
||||
the visible messages, and answer each new thread in full even when related \
|
||||
prior context appears here.\n\n";
|
||||
|
||||
/// Renders the personality roster for the master agent's system prompt.
|
||||
///
|
||||
/// When [`PromptContext::personality_roster`] is non-empty, emits an
|
||||
@@ -321,23 +346,33 @@ impl PromptSection for UserFilesSection {
|
||||
// Personality-specific MEMORY.md takes highest priority, then
|
||||
// the session-frozen curated-memory snapshot, then the
|
||||
// workspace file (pure prompt-unit tests and older call sites).
|
||||
//
|
||||
// Render into a scratch buffer first so the `MEMORY_MD_FRAMING`
|
||||
// note is only emitted when the block actually carries content —
|
||||
// the inject helpers silently skip empty/missing files, and a
|
||||
// dangling frame pointing at nothing would be worse than none.
|
||||
let mut mem = String::new();
|
||||
if let Some(ref memory_md) = ctx.personality_memory_md {
|
||||
tracing::debug!(
|
||||
"[user_files] personality MEMORY.md override active ({} chars)",
|
||||
memory_md.len()
|
||||
);
|
||||
inject_inline_content(&mut out, "MEMORY.md", memory_md, USER_FILE_MAX_CHARS);
|
||||
inject_inline_content(&mut mem, "MEMORY.md", memory_md, USER_FILE_MAX_CHARS);
|
||||
} else if let Some(snap) = &ctx.curated_snapshot {
|
||||
inject_snapshot_content(&mut out, "MEMORY.md", &snap.memory, USER_FILE_MAX_CHARS);
|
||||
inject_snapshot_content(&mut out, "USER.md", &snap.user, USER_FILE_MAX_CHARS);
|
||||
inject_snapshot_content(&mut mem, "MEMORY.md", &snap.memory, USER_FILE_MAX_CHARS);
|
||||
inject_snapshot_content(&mut mem, "USER.md", &snap.user, USER_FILE_MAX_CHARS);
|
||||
} else {
|
||||
inject_workspace_file_capped(
|
||||
&mut out,
|
||||
&mut mem,
|
||||
ctx.workspace_dir,
|
||||
"MEMORY.md",
|
||||
USER_FILE_MAX_CHARS,
|
||||
);
|
||||
}
|
||||
if !mem.trim().is_empty() {
|
||||
out.push_str(MEMORY_MD_FRAMING);
|
||||
out.push_str(&mem);
|
||||
}
|
||||
}
|
||||
Ok(out)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user