From f8ab967785d388c55b6eefd2fa68938b5cedd996 Mon Sep 17 00:00:00 2001 From: Ruslan Date: Tue, 3 Dec 2024 19:45:09 +0200 Subject: [PATCH] READY : Agents (#1500) * `parent` * Simplify context * Remove redunant `from_iter` * Fix joining of paths * Start working on `ScenarioProcessed` * Implement `ScenarioProcessed` --- module/move/assistant/Cargo.toml | 1 - module/move/assistant/src/agents.rs | 1 + module/move/assistant/src/agents/context.rs | 72 ++------ module/move/assistant/src/agents/path.rs | 160 +++++------------- .../src/agents/scenario_processed.rs | 99 +++++++++++ .../tests/inc/agents_tests/context_test.rs | 19 +-- .../assistant/tests/inc/agents_tests/mod.rs | 3 +- .../tests/inc/agents_tests/path_test.rs | 79 +-------- .../agents_tests/scenario_processed_test.rs | 25 +++ .../tests/inc/agents_tests/test_scenarios.rs | 23 +++ 10 files changed, 218 insertions(+), 264 deletions(-) create mode 100644 module/move/assistant/src/agents/scenario_processed.rs create mode 100644 module/move/assistant/tests/inc/agents_tests/scenario_processed_test.rs diff --git a/module/move/assistant/Cargo.toml b/module/move/assistant/Cargo.toml index 1031eed817..50700f8c3c 100644 --- a/module/move/assistant/Cargo.toml +++ b/module/move/assistant/Cargo.toml @@ -57,7 +57,6 @@ serde_with = "3.11.0" error_tools = "0.17.0" derive_tools = { version = "0.32.0", features = ["full"] } regex = { version = "1.10.3" } -itertools = "0.13.0" serde_yaml = "0.9" [dev-dependencies] diff --git a/module/move/assistant/src/agents.rs b/module/move/assistant/src/agents.rs index c902864e0a..0f04d8a61c 100644 --- a/module/move/assistant/src/agents.rs +++ b/module/move/assistant/src/agents.rs @@ -11,5 +11,6 @@ crate::mod_interface! layer context; layer scenario_raw; layer scenario_raw_processors; + layer scenario_processed; } \ No newline at end of file diff --git a/module/move/assistant/src/agents/context.rs b/module/move/assistant/src/agents/context.rs index 27969f0ef4..a18a535780 100644 --- a/module/move/assistant/src/agents/context.rs +++ b/module/move/assistant/src/agents/context.rs @@ -9,62 +9,14 @@ mod private use std::collections::HashMap; use crate::*; - use agents::path::Path; - - /// Simplistic in-memory "filesystem". Represents the root of the filesystem. - /// - /// `T` is the type of terminal object. - #[ derive( Debug, Default ) ] - pub struct Context< T > - { - root : ContextDir< T >, - } - - impl< T > Context< T > + use agents::path:: { - /// Create an empty `Context`. - pub fn new() -> Self - { - Self - { - root : ContextDir::new() - } - } - - /// Add new entry to the directory. - /// - /// Returns `true` if entry was successfully added. - /// Returns `false` if there is already and entry with such name. - /// Old entry will not be overriden. - pub fn add( &mut self, name : impl Into< String >, entry : ContextEntry< T > ) -> bool - { - self.root.add( name, entry ) - } - - /// Get an entry by its name. Returns `None` is there is no such entry. - /// - /// `name` must be a valid path item. Refer to `path::PATH_ITEM_REGEX_STR` for syntax. - /// - /// This method is useful for quickly getting an entry only by its name. - /// For complex paths, where your object is located in several consecutives directories, - /// you can use `Path` type and use method `Context::get_by_path`. - pub fn get( &self, name : impl AsRef< str > ) -> Option< &ContextEntry< T > > - { - self.root.get( name ) - } - - /// Get an entry by its path. Returns `None` is there is no such entry. - /// - /// This function can accept absolute `Path`s as `Context` represents the root of the - /// filesystem. - pub fn get_by_path( &self, path : &Path ) -> Option< &ContextEntry< T > > - { - self.root.get_by_path( &path.remove_absolute() ) - } - } + Path, + PATH_SEPARATOR, + }; - /// Represents a directory in `Context` with other directories and - /// terminal objects. + /// Represents a directory in a simplistic in-memory "filesystem" + /// with other directories and terminal objects. /// /// `T` is the type of terminal object. #[ derive( Debug, PartialEq, Clone, Default ) ] @@ -119,14 +71,19 @@ mod private /// Get an entry by its path. Returns `None` is there is no such entry. /// - /// This function does not accept absolute `Path`, as `ContextDir` does not know - /// whether it is root or not. For absolute `Path`s use `Context::get_by_path`. + /// This function accepts both relative and absolute paths and it will + /// treat itself as the root. pub fn get_by_path( &self, path : &Path ) -> Option< &ContextEntry< T > > { let mut cur : Option< &ContextEntry< T > > = None; for component in path.components() { + if component == PATH_SEPARATOR + { + continue; + } + match cur { None => @@ -161,7 +118,7 @@ mod private } } - /// Entry in `Context`: either a directory or a terminal object `T`. + /// Entry in a simplistic in-memory "filesystem": either a directory or a terminal object `T`. /// /// Notice, this struct does not store the name of the entry. #[ derive( Debug, PartialEq, Clone ) ] @@ -187,7 +144,6 @@ crate::mod_interface! { own use { - Context, ContextDir, ContextEntry, }; diff --git a/module/move/assistant/src/agents/path.rs b/module/move/assistant/src/agents/path.rs index 2959e94ea0..d0c29a15dd 100644 --- a/module/move/assistant/src/agents/path.rs +++ b/module/move/assistant/src/agents/path.rs @@ -12,7 +12,12 @@ mod private sync::LazyLock, }; - use itertools::Itertools; + use serde:: + { + Serialize, + Deserialize, + }; + use regex::Regex; /// Path separator string. @@ -24,20 +29,6 @@ mod private /// If you want to match against this expression, use `PATH_ITEM_REGEX`. pub const PATH_ITEM_REGEX_STR : &str = r"[a-zA-Z0-9_ -]+"; - /// Regular expression for `Path` items. You can match whole `&str` with this type. - /// - /// To match whole `Path` in strings, use `PATH_REGEX`. - pub static PATH_ITEM_REGEX : LazyLock< Regex > = LazyLock::new( || - { - let regex = format! - ( - r"^{}$", - PATH_ITEM_REGEX_STR - ); - - Regex::new( ®ex ).unwrap() - }); - /// Regular expression for `Path`. You can match whole `&str` with this type. pub static PATH_REGEX : LazyLock< Regex > = LazyLock::new( || { @@ -56,7 +47,7 @@ mod private /// /// Paths resemble filesystem path, path separator is `::`. /// Absolute path starts with `::`. - #[ derive( Debug, Clone, Eq, PartialEq, Hash ) ] + #[ derive( Debug, Clone, Eq, PartialEq, Hash, Serialize, Deserialize ) ] pub struct Path( String ); impl Path @@ -67,6 +58,34 @@ mod private #[ inline ] pub fn parent( &self ) -> Option< Path > { + /// Find parent of a `Path`. + /// + /// This method uses `&str` as an argument instead of `Path` + /// in order to be more general and handle trailing `::` case. + fn find_parent( s : &str ) -> Option< &str > + { + s.rfind( PATH_SEPARATOR ) + .map( | sep_pos | + { + if sep_pos == 0 + { + // We found root. We should not return string before `::`, + // as it will be empty. + Some( PATH_SEPARATOR ) + } + else if sep_pos == s.len() - PATH_SEPARATOR.len() + { + // We found trailing `::`. We should continue looking for last separator. + find_parent( &s[ .. sep_pos ] ) + } + else + { + Some( &s[ .. sep_pos ] ) + } + }) + .flatten() + } + find_parent( self.0.as_str() ) .map( | s | Self( s.to_string() ) ) } @@ -83,41 +102,26 @@ mod private self.0.starts_with( PATH_SEPARATOR ) } - /// Turn an absolute `Path` into a relative one by removing leading `::`. - /// - /// If the `Path` is not absolute, a clone will be returned without any - /// changes. - pub fn remove_absolute( &self ) -> Path - { - if self.is_absolute() - { - Self( self.0.strip_prefix( PATH_SEPARATOR ).unwrap_or( "" ).to_string() ) - } - else - { - Self( self.0.clone() ) - } - } - /// Creates an owned `Path` by joining a given path to `self`. /// - /// Returns `Err(io::Error)` is the `path` is an absolute path. + /// If path is joined with an absolute path, then this absolute + /// path will be returned. #[ inline ] - pub fn join( &self, path : &Path ) -> Result< Self, io::Error > + pub fn join( &self, path : &Path ) -> Self { if path.is_absolute() { - Err( io::Error::from( io::ErrorKind::InvalidData ) ) + path.clone() } else { if self.0.ends_with( PATH_SEPARATOR ) { - Ok( Self( format!( "{}{}", self.0, path.0 ) ) ) + Self( format!( "{}{}", self.0, path.0 ) ) } else { - Ok( Self( format!( "{}::{}", self.0, path.0 ) ) ) + Self( format!( "{}::{}", self.0, path.0 ) ) } } } @@ -136,54 +140,6 @@ mod private self.0 } - /// Creates a relative `Path` from an iterator over items that implement `AsRef`. - /// To create an absolute `Path`, use `from_iter_abs` method. - /// - /// Returns `Err(io::Error)` if the items are not valid `Path` items. - pub fn from_iter_rel< 'a >( iter : impl Iterator< Item = &'a str > ) -> Result< Self, io::Error > - { - iter.map( | path_element_str | - { - if PATH_ITEM_REGEX.is_match( path_element_str ) - { - Ok ( path_element_str ) - } - else - { - Err ( io::Error::from( io::ErrorKind::InvalidData ) ) - } - }) - .process_results( | mut item_iter | - { - Self( item_iter.join( PATH_SEPARATOR ) ) - }) - } - - /// Creates an absolute `Path` from an iterator over strings. - /// To create a relative `Path`, use `from_iter_rel` method. - /// - /// Returns `Err(io::Error)` if the items are not valid `Path` items. - pub fn from_iter_abs< 'a >( iter : impl Iterator< Item = &'a str > ) -> Result< Self, io::Error > - { - iter.map( | path_element_str | - { - if PATH_ITEM_REGEX.is_match( path_element_str ) - { - Ok ( path_element_str ) - } - else - { - Err ( io::Error::from( io::ErrorKind::InvalidData ) ) - } - }) - .process_results( | mut item_iter | - { - let mut res = item_iter.join( PATH_SEPARATOR ); - res.insert_str( 0, PATH_SEPARATOR ); - Self( res ) - }) - } - /// Iterate over components of a `Path`. If the `Path` is absolute, then the first /// element will be `::`. pub fn components( &self ) -> impl Iterator< Item = &str > @@ -202,34 +158,6 @@ mod private } } - /// Find parent of a `Path`. - /// - /// This method uses `&str` as an argument instead of `Path` - /// in order to be more general and handle trailing `::` case. - fn find_parent( s : &str ) -> Option< &str > - { - s.rfind( PATH_SEPARATOR ) - .map( | sep_pos | - { - if sep_pos == 0 - { - // We found root. We should not return string before `::`, - // as it will be empty. - Some( PATH_SEPARATOR ) - } - else if sep_pos == s.len() - PATH_SEPARATOR.len() - { - // We found trailing `::`. We should continue looking for last separator. - find_parent( &s[ .. sep_pos ] ) - } - else - { - Some( &s[ .. sep_pos ] ) - } - }) - .flatten() - } - impl fmt::Display for Path { #[ inline ] @@ -305,5 +233,9 @@ mod private crate::mod_interface! { - own use Path; + own use + { + Path, + PATH_SEPARATOR, + }; } \ No newline at end of file diff --git a/module/move/assistant/src/agents/scenario_processed.rs b/module/move/assistant/src/agents/scenario_processed.rs new file mode 100644 index 0000000000..b43e9c0075 --- /dev/null +++ b/module/move/assistant/src/agents/scenario_processed.rs @@ -0,0 +1,99 @@ +//! +//! Scenario representation. Stores parsed representation of templates and paths. +//! This is the type used for running scenarios. +//! +//! For a more simplistic representation use `ScenarioRaw`. +//! + +mod private +{ + use std::collections::HashMap; + + use crate::*; + use agents:: + { + path::Path, + scenario_raw:: + { + ScenarioRaw, + NodeRaw, + }, + }; + + /// New type for templates in scenarios. + #[ derive( Debug, PartialEq ) ] + pub struct TemplateBody( pub String ); + + /// Struct that represents user written scenarios. + /// + /// This is a processed form of a scenario, paths are distinguished here with types. + /// For more simplistic representation of scenarios, use `ScenarioRaw` type. + #[ derive( Debug, PartialEq ) ] + pub struct ScenarioProcessed + { + /// Nodes in the scenario. + pub nodes: Vec< Node >, + } + + impl TryFrom< ScenarioRaw > for ScenarioProcessed + { + type Error = std::io::Error; + + fn try_from( scenario_raw : ScenarioRaw ) -> Result< Self, Self::Error > + { + let nodes : Result< Vec< Node >, Self::Error > = + scenario_raw.nodes.into_iter().map( | rn | Node::try_from( rn ) ).collect(); + + Ok( Self { nodes : nodes? } ) + } + } + + /// Node representation in a scenario file. + /// + /// This is a processed form of a node, paths are distinguished here with types. + /// For more simplistic representation of scenarios, use `NodeRaw` type. + #[ derive( Debug, PartialEq ) ] + pub struct Node + { + /// ID of the node. Must be unique, will also identify node output. + pub id : String, + + /// Type of the node. + pub r#type : Path, + + /// Parameters of the node. + pub params : HashMap< String, String >, + + /// ID of the next node to execute. + pub next : Path, + } + + impl TryFrom< NodeRaw > for Node + { + type Error = std::io::Error; + + fn try_from( node_raw : NodeRaw ) -> Result< Self, Self::Error > + { + Ok + ( + Self + { + id : node_raw.id, + r#type : Path::try_from( node_raw.r#type )?, + params : node_raw.params, + next : Path::try_from( node_raw.next )?, + } + ) + } + } +} + +crate::mod_interface! +{ + own use + { + TemplateBody, + ScenarioProcessed, + Node, + }; +} \ No newline at end of file diff --git a/module/move/assistant/tests/inc/agents_tests/context_test.rs b/module/move/assistant/tests/inc/agents_tests/context_test.rs index 08b0461696..e28fc8c264 100644 --- a/module/move/assistant/tests/inc/agents_tests/context_test.rs +++ b/module/move/assistant/tests/inc/agents_tests/context_test.rs @@ -7,7 +7,6 @@ use the_module::agents:: { ContextDir, ContextEntry, - Context, }, }; @@ -108,12 +107,14 @@ fn context_dir_get_by_path_relative() #[ test ] fn context_dir_get_by_path_absolute() { + let entry = ContextEntry::Terminal( () ); let mut ctx : ContextDir< () > = ContextDir::new(); - ctx.add( "test", ContextEntry::Terminal( () ) ); + ctx.add( "test", entry.clone() ); let res = ctx.get_by_path( &&Path::try_from( "::test" ).unwrap() ); - assert!( res.is_none() ); + assert!( res.is_some() ); + assert_eq!( res.unwrap(), &entry ); } #[ test ] @@ -124,16 +125,4 @@ fn context_dir_get_by_path_non_existing() let res = ctx.get_by_path( &Path::try_from( "test" ).unwrap() ); assert!( res.is_none() ); -} - -#[ test ] -fn context_get_by_path_absolute() -{ - let mut ctx : Context< () > = Context::new(); - let entry = ContextEntry::Terminal( () ); - ctx.add( "test", entry.clone() ); - - let res = ctx.get_by_path( &Path::try_from( "::test" ).unwrap() ); - - assert_eq!( res, Some( &entry ) ); } \ No newline at end of file diff --git a/module/move/assistant/tests/inc/agents_tests/mod.rs b/module/move/assistant/tests/inc/agents_tests/mod.rs index 6c94bd4a2e..f4260d9ed5 100644 --- a/module/move/assistant/tests/inc/agents_tests/mod.rs +++ b/module/move/assistant/tests/inc/agents_tests/mod.rs @@ -5,4 +5,5 @@ mod test_scenarios; mod path_test; mod context_test; mod scenario_raw_test; -mod scenario_raw_processors; \ No newline at end of file +mod scenario_raw_processors; +mod scenario_processed_test; \ No newline at end of file diff --git a/module/move/assistant/tests/inc/agents_tests/path_test.rs b/module/move/assistant/tests/inc/agents_tests/path_test.rs index 78e4132502..277f4965ff 100644 --- a/module/move/assistant/tests/inc/agents_tests/path_test.rs +++ b/module/move/assistant/tests/inc/agents_tests/path_test.rs @@ -135,8 +135,7 @@ fn path_join_relative() let combined = orig_path.join( &append ); - assert!( combined.is_ok() ); - assert_eq!( combined.unwrap().inner(), "agent::completion" ); + assert_eq!( combined.inner(), "agent::completion" ); } #[ test ] @@ -147,7 +146,7 @@ fn path_join_absolute() let combined = orig_path.join( &append ); - assert!( combined.is_err() ); + assert_eq!( combined.inner(), "::completion" ); } #[ test ] @@ -158,8 +157,7 @@ fn path_join_root() let combined = orig_path.join( &append ); - assert!( combined.is_ok() ); - assert_eq!( combined.unwrap().inner(), "::agent" ); + assert_eq!( combined.inner(), "::agent" ); } #[ test ] @@ -170,8 +168,7 @@ fn path_join_trailing() let combined = orig_path.join( &append ); - assert!( combined.is_ok() ); - assert_eq!( combined.unwrap().inner(), "agents::completion" ); + assert_eq!( combined.inner(), "agents::completion" ); } #[ test ] @@ -251,74 +248,6 @@ fn path_inner() assert_eq!( inner, path_str ); } -#[ test ] -fn path_from_iter_right() -{ - let expected = "agents::completion"; - let elements = vec![ "agents", "completion" ]; - - let path = Path::from_iter_rel( elements.into_iter() ); - - assert!( path.is_ok() ); - let path = path.unwrap(); - assert!( path.is_relative() ); - assert_eq!( path.inner(), expected ); -} - -#[ test ] -fn path_from_iter_wrong_item() -{ - let elements = vec![ "agents:", "completion" ]; - - let path = Path::from_iter_rel( elements.into_iter() ); - - assert!( path.is_err() ); -} - -#[ test ] -fn path_from_iter_wrong_separator() -{ - let elements = vec![ "agents", "::", "completion" ]; - - let path = Path::from_iter_rel( elements.into_iter() ); - - assert!( path.is_err() ); -} - -#[ test ] -fn path_from_iter_abs() -{ - let expected = "::agents::completion"; - let elements = vec![ "agents", "completion" ]; - - let path = Path::from_iter_abs( elements.into_iter() ); - - assert!( path.is_ok() ); - let path = path.unwrap(); - assert!( path.is_absolute() ); - assert_eq!( path.inner(), expected ); -} - -#[ test ] -fn path_remove_absolute() -{ - let path = Path::try_from( "::agents::completion" ).unwrap(); - - let got_path = path.remove_absolute(); - - assert_eq!( got_path.inner(), "agents::completion" ); -} - -#[ test ] -fn path_remove_absolute_from_rel() -{ - let path = Path::try_from( "agents::completion" ).unwrap(); - - let got_path = path.remove_absolute(); - - assert_eq!( got_path.inner(), "agents::completion" ); -} - #[ test ] fn path_components() { diff --git a/module/move/assistant/tests/inc/agents_tests/scenario_processed_test.rs b/module/move/assistant/tests/inc/agents_tests/scenario_processed_test.rs new file mode 100644 index 0000000000..5fc734ae41 --- /dev/null +++ b/module/move/assistant/tests/inc/agents_tests/scenario_processed_test.rs @@ -0,0 +1,25 @@ +use super::*; + +use the_module::agents::scenario_processed::ScenarioProcessed; + +use test_scenarios:: +{ + gen_test_scenario_raw, + gen_test_scenario_raw_wrong, +}; + +#[ test ] +fn scenario_processed_right() +{ + let scenario_processed = ScenarioProcessed::try_from( gen_test_scenario_raw() ); + + assert!( scenario_processed.is_ok() ); +} + +#[ test ] +fn scenario_processed_wrong() +{ + let scenario_processed = ScenarioProcessed::try_from( gen_test_scenario_raw_wrong() ); + + assert!( scenario_processed.is_err() ); +} \ No newline at end of file diff --git a/module/move/assistant/tests/inc/agents_tests/test_scenarios.rs b/module/move/assistant/tests/inc/agents_tests/test_scenarios.rs index 84e217b230..02433a68ea 100644 --- a/module/move/assistant/tests/inc/agents_tests/test_scenarios.rs +++ b/module/move/assistant/tests/inc/agents_tests/test_scenarios.rs @@ -6,6 +6,7 @@ use the_module::agents::scenario_raw:: NodeRaw, }; +/// Generates an example `ScenarioRaw`. pub fn gen_test_scenario_raw() -> ScenarioRaw { ScenarioRaw::former() @@ -38,4 +39,26 @@ pub fn gen_test_scenario_raw() -> ScenarioRaw .form(), ] ) .form() +} + +/// Generates a `ScenarioRaw` with wrong syntax for `Path`. +pub fn gen_test_scenario_raw_wrong() -> ScenarioRaw +{ + ScenarioRaw::former() + .nodes( vec! + [ + NodeRaw::former() + .id( "node_1".to_string() ) + .r#type( ":agents:".to_string() ) // This part is incorrect. Path written in wrong syntax. + .params( + { + let mut map : HashMap< String, String > = HashMap::new(); + map.insert( "model".into(), "gpt-4o-mini".into() ); + map + } + ) + .next( "node_2".to_string() ) + .form(), + ] ) + .form() } \ No newline at end of file