From b1e97d68fbe2072c8b680c43f4570a7f4eefdaae Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Thu, 27 Aug 2026 15:32:50 +0700 Subject: [PATCH] fix(prompt): append the tool-policy boundary instead of prepending it (#5704) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `build_system_prompt` put the `## Tool Policy Boundary` block in front of the whole assembled prompt. Every line of that block is session-scoped — agent id, channel, entry point, risk level, allowed permission, the allowed-tool list, a restricted count — so the prompt's first diverging byte sat at offset 0 and the inference backend's automatic prefix cache had nothing to reuse behind it, even when the safety preamble, tool catalogue and workspace sections were byte-identical. That contradicts what the surrounding code already protects: `SystemPromptBuilder::for_subagent` keeps `DateTimeSection` out precisely to avoid one varying value inside the prompt, and the connected-server overview is sorted so it "does not reshuffle between turns and cost its cached prefix". It also restores the archetype/persona as the prompt's opening line. The prepend replaced it with the same constant heading for every agent, which is a silent layout change for anything that identified an agent that way. The composition moved into a pure `append_tool_policy_boundary(prompt, boundary)` so the ordering is testable without standing up a session. Four tests, three of which fail against the prepend: - the boundary lands after the prompt body - the persona stays the opening line - two agents differing only in the boundary share the whole body as a common leading prefix (this is the cache property itself, not a proxy) - no boundary leaves the prompt untouched (passes either way) `cargo test --lib turn::context` 4 passed; reverting the helper to prepend gives 1 passed / 3 failed. `cargo fmt --all` clean. --- .../agent/harness/session/turn/context.rs | 93 ++++++++++++++++++- 1 file changed, 88 insertions(+), 5 deletions(-) diff --git a/src/openhuman/agent/harness/session/turn/context.rs b/src/openhuman/agent/harness/session/turn/context.rs index 9281bb440b..e6b53c180b 100644 --- a/src/openhuman/agent/harness/session/turn/context.rs +++ b/src/openhuman/agent/harness/session/turn/context.rs @@ -335,10 +335,93 @@ impl Agent { // Route through the global context manager so every // prompt-building call-site — main agent, sub-agent runner, // channel runtimes — shares one builder configuration. - let mut prompt = self.context.build_system_prompt(&ctx)?; - if let Some(boundary) = render_tool_policy_boundary(&self.tool_policy_session, 2048) { - prompt = format!("{boundary}\n\n{prompt}"); - } - Ok(prompt) + let prompt = self.context.build_system_prompt(&ctx)?; + // Appended, not prepended (#5704). Every line of this block is + // session-scoped — agent id, channel, entry point, risk level, the + // allowed-tool list — so putting it first moves the prompt's first + // diverging byte to offset 0 and costs the inference backend's + // automatic prefix cache everything behind it. That is the same + // concern that keeps DateTimeSection out of `for_subagent` and keeps + // the connected-server overview sorted. The model reads the whole + // system message either way. + // + // It also keeps the archetype/persona as the prompt's opening line, + // which the prepend had replaced with a constant heading for every + // agent. + let boundary = render_tool_policy_boundary(&self.tool_policy_session, 2048); + Ok(append_tool_policy_boundary(prompt, boundary)) + } +} + +/// Place the tool-policy boundary block relative to the assembled prompt. +/// +/// Separated from [`Agent`] so the ordering can be tested without standing up a +/// session: everything that decides the placement is in these two arguments. +fn append_tool_policy_boundary(prompt: String, boundary: Option) -> String { + match boundary { + Some(boundary) => format!("{prompt}\n\n{boundary}"), + None => prompt, + } +} + +#[cfg(test)] +mod tool_policy_boundary_placement_tests { + use super::append_tool_policy_boundary; + + const PERSONA: &str = "You are the archetype.\nMore persona."; + const BOUNDARY: &str = "## Tool Policy Boundary\n- Agent: alpha"; + + #[test] + fn the_boundary_goes_after_the_prompt_body() { + let out = append_tool_policy_boundary(PERSONA.into(), Some(BOUNDARY.into())); + let body_at = out.find("You are the archetype.").expect("body present"); + let boundary_at = out + .find("## Tool Policy Boundary") + .expect("boundary present"); + assert!( + body_at < boundary_at, + "the session-scoped block must not precede the stable prompt (#5704):\n{out}" + ); + } + + #[test] + fn the_persona_stays_the_opening_line() { + let out = append_tool_policy_boundary(PERSONA.into(), Some(BOUNDARY.into())); + assert_eq!( + out.lines().next(), + Some("You are the archetype."), + "prepending replaced every agent's first line with a constant heading" + ); + } + + #[test] + fn two_agents_share_the_whole_prompt_body_as_a_common_prefix() { + // The point of appending: the varying part is last, so everything the + // two turns have in common is a shared leading prefix the backend can + // reuse. Prepending moved the first diverging byte to offset 0. + let alpha = append_tool_policy_boundary( + PERSONA.into(), + Some("## Tool Policy Boundary\n- Agent: alpha".into()), + ); + let beta = append_tool_policy_boundary( + PERSONA.into(), + Some("## Tool Policy Boundary\n- Agent: beta".into()), + ); + let shared = alpha + .bytes() + .zip(beta.bytes()) + .take_while(|(a, b)| a == b) + .count(); + assert!( + shared >= PERSONA.len(), + "the shared prefix ({shared} bytes) must cover the whole stable body ({} bytes)", + PERSONA.len() + ); + } + + #[test] + fn no_boundary_leaves_the_prompt_untouched() { + let out = append_tool_policy_boundary(PERSONA.into(), None); + assert_eq!(out, PERSONA); } }