Skip to content

Commit e9ca2bc

Browse files
feat: Stable system prompt and tool ordering for more cache hits (#5192)
1 parent bbf9d30 commit e9ca2bc

2 files changed

Lines changed: 103 additions & 1 deletion

File tree

crates/goose/src/agents/prompt_manager.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ impl PromptManager {
2525
system_prompt_override: None,
2626
system_prompt_extras: Vec::new(),
2727
// Use the fixed current date time so that prompt cache can be used.
28-
current_date_timestamp: Utc::now().format("%Y-%m-%d %H:%M:%S").to_string(),
28+
// Filtering to an hour to balance user time accuracy and multi session prompt cache hits.
29+
current_date_timestamp: Utc::now().format("%Y-%m-%d %H:00").to_string(),
2930
}
3031
}
3132

@@ -58,6 +59,8 @@ impl PromptManager {
5859
false,
5960
));
6061
}
62+
// Stable tool ordering is important for multi session prompt caching.
63+
extensions_info.sort_by(|a, b| a.name.cmp(&b.name));
6164

6265
let sanitized_extensions_info: Vec<ExtensionInfo> = extensions_info
6366
.into_iter()

crates/goose/src/agents/reply_parts.rs

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@ impl Agent {
6060
tools.push(frontend_tool.tool.clone());
6161
}
6262

63+
if !router_enabled {
64+
// Stable tool ordering is important for multi session prompt caching.
65+
tools.sort_by(|a, b| a.name.cmp(&b.name));
66+
}
67+
6368
// Prepare system prompt
6469
let extensions_info = self.extension_manager.get_extensions_info().await;
6570

@@ -282,3 +287,97 @@ impl Agent {
282287
Ok(())
283288
}
284289
}
290+
291+
#[cfg(test)]
292+
mod tests {
293+
use super::*;
294+
use crate::conversation::message::Message;
295+
use crate::model::ModelConfig;
296+
use crate::providers::base::{Provider, ProviderUsage, Usage};
297+
use crate::providers::errors::ProviderError;
298+
use async_trait::async_trait;
299+
use rmcp::object;
300+
301+
#[derive(Clone)]
302+
struct MockProvider {
303+
model_config: ModelConfig,
304+
}
305+
306+
#[async_trait]
307+
impl Provider for MockProvider {
308+
fn metadata() -> crate::providers::base::ProviderMetadata {
309+
crate::providers::base::ProviderMetadata::empty()
310+
}
311+
312+
fn get_model_config(&self) -> ModelConfig {
313+
self.model_config.clone()
314+
}
315+
316+
async fn complete_with_model(
317+
&self,
318+
_model_config: &ModelConfig,
319+
_system: &str,
320+
_messages: &[Message],
321+
_tools: &[Tool],
322+
) -> anyhow::Result<(Message, ProviderUsage), ProviderError> {
323+
Ok((
324+
Message::assistant().with_text("ok"),
325+
ProviderUsage::new("mock".to_string(), Usage::default()),
326+
))
327+
}
328+
}
329+
330+
#[tokio::test]
331+
async fn prepare_tools_sorts_when_router_disabled_and_includes_frontend_and_list_tools(
332+
) -> anyhow::Result<()> {
333+
let agent = crate::agents::Agent::new();
334+
335+
let model_config = ModelConfig::new("test-model").unwrap();
336+
let provider = std::sync::Arc::new(MockProvider { model_config });
337+
agent.update_provider(provider).await?;
338+
339+
// Disable the router to trigger sorting
340+
agent.disable_router_for_recipe().await;
341+
342+
// Add unsorted frontend tools
343+
let frontend_tools = vec![
344+
Tool::new(
345+
"frontend__z_tool".to_string(),
346+
"Z tool".to_string(),
347+
object!({ "type": "object", "properties": { } }),
348+
),
349+
Tool::new(
350+
"frontend__a_tool".to_string(),
351+
"A tool".to_string(),
352+
object!({ "type": "object", "properties": { } }),
353+
),
354+
];
355+
356+
agent
357+
.add_extension(crate::agents::extension::ExtensionConfig::Frontend {
358+
name: "frontend".to_string(),
359+
description: "desc".to_string(),
360+
tools: frontend_tools,
361+
instructions: None,
362+
bundled: None,
363+
available_tools: vec![],
364+
})
365+
.await
366+
.unwrap();
367+
368+
let (tools, _toolshim_tools, _system_prompt) = agent.prepare_tools_and_prompt().await?;
369+
370+
// Ensure both platform and frontend tools are present
371+
let names: Vec<String> = tools.iter().map(|t| t.name.clone().into_owned()).collect();
372+
assert!(names.iter().any(|n| n.starts_with("platform__")));
373+
assert!(names.iter().any(|n| n == "frontend__a_tool"));
374+
assert!(names.iter().any(|n| n == "frontend__z_tool"));
375+
376+
// Verify the names are sorted ascending
377+
let mut sorted = names.clone();
378+
sorted.sort();
379+
assert_eq!(names, sorted);
380+
381+
Ok(())
382+
}
383+
}

0 commit comments

Comments
 (0)