refactor: simplify memory trait requirements and remove unused tool hooks
This commit is contained in:
@@ -40,10 +40,6 @@ impl LlmApiAdaptor<LlmMessage> for LlmCompletionModel {
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 2. Tool validation hook — only allows known tools
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
struct ToolValidationHook {
|
||||
valid_names: Vec<String>,
|
||||
}
|
||||
@@ -74,7 +70,7 @@ struct LlmChatAgentBuilder {
|
||||
tools: Vec<Box<dyn ToolDyn>>,
|
||||
hooks: Vec<Box<dyn Hook>>,
|
||||
max_turns: usize,
|
||||
memory_data: Option<Box<dyn AgentMemory<LlmMessage> + Sync>>,
|
||||
memory_data: Option<Box<dyn AgentMemory<LlmMessage>>>,
|
||||
context: Option<serde_json::Value>,
|
||||
}
|
||||
|
||||
@@ -118,7 +114,7 @@ impl AgentBuilder<LlmMessage, LlmCompletionModel> for LlmChatAgentBuilder {
|
||||
self
|
||||
}
|
||||
|
||||
fn memory(mut self, memory: impl AgentMemory<LlmMessage> + Sync + 'static) -> Self {
|
||||
fn memory(mut self, memory: impl AgentMemory<LlmMessage> + 'static) -> Self {
|
||||
self.memory_data = Some(Box::new(memory));
|
||||
self
|
||||
}
|
||||
|
||||
@@ -12,7 +12,7 @@ pub struct Agent<M: LlmApiAdaptor<Message>, Message> {
|
||||
pub hooks: Vec<Box<dyn Hook>>,
|
||||
pub default_max_turns: usize,
|
||||
pub system_prompt: Option<Message>,
|
||||
pub memory: Option<Box<dyn AgentMemory<Message> + Sync>>,
|
||||
pub memory: Option<Box<dyn AgentMemory<Message>>>,
|
||||
pub context: Option<serde_json::Value>,
|
||||
}
|
||||
|
||||
@@ -43,7 +43,7 @@ where
|
||||
self
|
||||
}
|
||||
|
||||
pub fn memory(mut self, mem: impl AgentMemory<Message> + Sync + 'static) -> Self {
|
||||
pub fn memory(mut self, mem: impl AgentMemory<Message> + 'static) -> Self {
|
||||
self.memory = Some(Box::new(mem));
|
||||
self
|
||||
}
|
||||
@@ -62,50 +62,6 @@ where
|
||||
&self.tools
|
||||
}
|
||||
|
||||
pub fn call_tool<'s>(
|
||||
&'s self,
|
||||
name: &str,
|
||||
args: serde_json::Value,
|
||||
) -> Pin<Box<dyn Future<Output = Result<String, String>> + Send + 's>> {
|
||||
let tools: Vec<&Box<dyn ToolDyn>> = self.tools.iter().collect();
|
||||
let hooks: Vec<&Box<dyn Hook>> = self.hooks.iter().collect();
|
||||
let name = name.to_string();
|
||||
let context = self.context.clone().unwrap_or_default();
|
||||
Box::pin(async move {
|
||||
let result = {
|
||||
let mut found = None;
|
||||
for tool in tools {
|
||||
if tool.tool_name() == name {
|
||||
found = Some(tool.call_with_context(args, context).await);
|
||||
break;
|
||||
}
|
||||
}
|
||||
found.ok_or_else(|| format!("unknown tool: {name}"))?
|
||||
};
|
||||
|
||||
match result.outcome {
|
||||
ToolOutcome::Success => {
|
||||
let mut json_output = result.output;
|
||||
// Run after_tool_call hooks
|
||||
for hook in &hooks {
|
||||
match hook.after_tool_call(&name, &mut json_output) {
|
||||
Flow::Stop => {
|
||||
return Err("tool call output rejected by hook".to_string());
|
||||
}
|
||||
Flow::Skip => {
|
||||
json_output = serde_json::Value::Null;
|
||||
break;
|
||||
}
|
||||
Flow::Continue => {}
|
||||
}
|
||||
}
|
||||
serde_json::to_string(&json_output).map_err(|e| e.to_string())
|
||||
}
|
||||
ToolOutcome::Failure(failure) if failure.fatal => Err(failure.message),
|
||||
ToolOutcome::Failure(failure) => Ok(format!("error: {}", failure.message)),
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
pub fn prompt(&self, message: impl Into<Message> + Send) -> PromptRequest<'_, M, Message>
|
||||
where
|
||||
@@ -208,28 +164,4 @@ mod tests {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn call_tool_returns_recoverable_failure_as_model_visible_error() {
|
||||
let agent = Agent::new(TestModel).tool(FailingTool);
|
||||
|
||||
let output = agent
|
||||
.call_tool("fail", json!({"fatal": false}))
|
||||
.await
|
||||
.expect("recoverable tool failure should be model visible");
|
||||
|
||||
assert_eq!(output, "error: recoverable failure");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn call_tool_propagates_fatal_failure() {
|
||||
let agent = Agent::new(TestModel).tool(FailingTool);
|
||||
|
||||
let error = agent
|
||||
.call_tool("fail", json!({"fatal": true}))
|
||||
.await
|
||||
.expect_err("fatal tool failure should be propagated");
|
||||
|
||||
assert_eq!(error, "fatal failure");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,7 +12,7 @@ pub trait AgentBuilder<Message, M: LlmApiAdaptor<Message>> {
|
||||
fn tool(self, tool: impl Tool + Send + Sync + 'static) -> Self;
|
||||
fn add_hook(self, hook: impl Hook + 'static) -> Self;
|
||||
fn max_turns(self, n: usize) -> Self;
|
||||
fn memory(self, memory: impl AgentMemory<Message> + Sync + 'static) -> Self;
|
||||
fn memory(self, memory: impl AgentMemory<Message> + 'static) -> Self;
|
||||
|
||||
fn context(self, context: serde_json::Value) -> Self;
|
||||
fn build(self) -> Agent<M, Message>;
|
||||
|
||||
@@ -1,4 +1,31 @@
|
||||
pub trait AgentMemory<Message> {
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
pub trait AgentMemory<Message>: Send + Sync {
|
||||
fn get_memory(&self) -> &[Message];
|
||||
fn append_message(&mut self, message: Message);
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct VecMemory<Message> {
|
||||
messages: Vec<Message>,
|
||||
}
|
||||
|
||||
impl<Message> VecMemory<Message> {
|
||||
pub fn new(messages: Vec<Message>) -> Self {
|
||||
Self { messages }
|
||||
}
|
||||
|
||||
pub fn into_inner(self) -> Vec<Message> {
|
||||
self.messages
|
||||
}
|
||||
}
|
||||
|
||||
impl<Message: Send + Sync> AgentMemory<Message> for VecMemory<Message> {
|
||||
fn get_memory(&self) -> &[Message] {
|
||||
&self.messages
|
||||
}
|
||||
|
||||
fn append_message(&mut self, message: Message) {
|
||||
self.messages.push(message);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -165,17 +165,17 @@ where
|
||||
let args = tc.args.clone();
|
||||
let context = self.context.clone().unwrap_or_default();
|
||||
let fut = async move {
|
||||
for tool in tools {
|
||||
if tool.tool_name() == name {
|
||||
return tool
|
||||
.call_with_context(args, context.clone())
|
||||
.await;
|
||||
match tools.iter().find(|tool| tool.tool_name() == name) {
|
||||
Some(tool) => {
|
||||
tool.call_with_context(args, context.clone()).await
|
||||
}
|
||||
None => ToolExecutionResult::failed(
|
||||
Value::Null,
|
||||
ToolFailure::invalid_args(format!(
|
||||
"unknown tool: {name}"
|
||||
)),
|
||||
),
|
||||
}
|
||||
ToolExecutionResult::failed(
|
||||
Value::Null,
|
||||
ToolFailure::invalid_args(format!("unknown tool: {name}")),
|
||||
)
|
||||
};
|
||||
fut.await
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user