From f32a4b3e629741cc5e176a2763672b4232590add Mon Sep 17 00:00:00 2001 From: Marcelle Bonterre Date: Thu, 6 Mar 2025 13:51:13 -0500 Subject: [PATCH 1/8] bufix workdirs. refactor to be async-safe, and simpler --- Cargo.lock | 86 ++++++----- crates/goose-bench/src/bench_work_dir.rs | 134 ++++++++++++++++++ .../src/eval_suites/core/create_file.rs | 4 +- .../src/eval_suites/core/example.rs | 4 +- .../goose-bench/src/eval_suites/core/image.rs | 4 +- .../src/eval_suites/core/list_files.rs | 4 +- .../src/eval_suites/core/save_fact.rs | 4 +- .../src/eval_suites/core/script.rs | 4 +- .../src/eval_suites/core/search_replace.rs | 10 +- .../src/eval_suites/core/web_scrape.rs | 4 +- .../goose-bench/src/eval_suites/evaluation.rs | 4 +- crates/goose-bench/src/lib.rs | 2 +- crates/goose-bench/src/work_dir.rs | 113 --------------- crates/goose-cli/src/commands/bench.rs | 77 +++++----- 14 files changed, 244 insertions(+), 210 deletions(-) create mode 100644 crates/goose-bench/src/bench_work_dir.rs delete mode 100644 crates/goose-bench/src/work_dir.rs diff --git a/Cargo.lock b/Cargo.lock index 8ce0ef66c877..002cf20867b3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -287,9 +287,9 @@ dependencies = [ [[package]] name = "aws-config" -version = "1.5.17" +version = "1.5.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "490aa7465ee685b2ced076bb87ef654a47724a7844e2c7d3af4e749ce5b875dd" +checksum = "90aff65e86db5fe300752551c1b015ef72b708ac54bded8ef43d0d53cb7cb0b1" dependencies = [ "aws-credential-types", "aws-runtime", @@ -297,7 +297,7 @@ dependencies = [ "aws-sdk-ssooidc", "aws-sdk-sts", "aws-smithy-async", - "aws-smithy-http", + "aws-smithy-http 0.61.1", "aws-smithy-json", "aws-smithy-runtime", "aws-smithy-runtime-api", @@ -336,7 +336,7 @@ dependencies = [ "aws-credential-types", "aws-sigv4", "aws-smithy-async", - "aws-smithy-http", + "aws-smithy-http 0.60.12", "aws-smithy-runtime", "aws-smithy-runtime-api", "aws-smithy-types", @@ -354,15 +354,15 @@ dependencies = [ [[package]] name = "aws-sdk-bedrockruntime" -version = "1.75.0" +version = "1.76.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2ddf7475b6f50a1a5be8edb1bcdf6e4ae00feed5b890d14a3f1f0e14d76f5a16" +checksum = "b538f72f5ab8d23de44aacd109788c37e268fe9f4d060168714a12514d73b434" dependencies = [ "aws-credential-types", "aws-runtime", "aws-smithy-async", "aws-smithy-eventstream", - "aws-smithy-http", + "aws-smithy-http 0.61.1", "aws-smithy-json", "aws-smithy-runtime", "aws-smithy-runtime-api", @@ -378,14 +378,14 @@ dependencies = [ [[package]] name = "aws-sdk-sso" -version = "1.60.0" +version = "1.61.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "60186fab60b24376d3e33b9ff0a43485f99efd470e3b75a9160c849741d63d56" +checksum = "e65ff295979977039a25f5a0bf067a64bc5e6aa38f3cef4037cf42516265553c" dependencies = [ "aws-credential-types", "aws-runtime", "aws-smithy-async", - "aws-smithy-http", + "aws-smithy-http 0.61.1", "aws-smithy-json", "aws-smithy-runtime", "aws-smithy-runtime-api", @@ -400,14 +400,14 @@ dependencies = [ [[package]] name = "aws-sdk-ssooidc" -version = "1.61.0" +version = "1.62.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7033130ce1ee13e6018905b7b976c915963755aef299c1521897679d6cd4f8ef" +checksum = "91430a60f754f235688387b75ee798ef00cfd09709a582be2b7525ebb5306d4f" dependencies = [ "aws-credential-types", "aws-runtime", "aws-smithy-async", - "aws-smithy-http", + "aws-smithy-http 0.61.1", "aws-smithy-json", "aws-smithy-runtime", "aws-smithy-runtime-api", @@ -422,14 +422,14 @@ dependencies = [ [[package]] name = "aws-sdk-sts" -version = "1.61.0" +version = "1.62.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c5c1cac7677179d622b4448b0d31bcb359185295dc6fca891920cfb17e2b5156" +checksum = "9276e139d39fff5a0b0c984fc2d30f970f9a202da67234f948fda02e5bea1dbe" dependencies = [ "aws-credential-types", "aws-runtime", "aws-smithy-async", - "aws-smithy-http", + "aws-smithy-http 0.61.1", "aws-smithy-json", "aws-smithy-query", "aws-smithy-runtime", @@ -450,7 +450,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9bfe75fad52793ce6dec0dc3d4b1f388f038b5eb866c8d4d7f3a8e21b5ea5051" dependencies = [ "aws-credential-types", - "aws-smithy-http", + "aws-smithy-http 0.60.12", "aws-smithy-runtime-api", "aws-smithy-types", "bytes", @@ -479,9 +479,9 @@ dependencies = [ [[package]] name = "aws-smithy-eventstream" -version = "0.60.6" +version = "0.60.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8b18559a41e0c909b77625adf2b8c50de480a8041e5e4a3f5f7d177db70abc5a" +checksum = "461e5e02f9864cba17cff30f007c2e37ade94d01e87cdb5204e44a84e6d38c17" dependencies = [ "aws-smithy-types", "bytes", @@ -493,6 +493,26 @@ name = "aws-smithy-http" version = "0.60.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7809c27ad8da6a6a68c454e651d4962479e81472aa19ae99e59f9aba1f9713cc" +dependencies = [ + "aws-smithy-runtime-api", + "aws-smithy-types", + "bytes", + "bytes-utils", + "futures-core", + "http 0.2.12", + "http-body 0.4.6", + "once_cell", + "percent-encoding", + "pin-project-lite", + "pin-utils", + "tracing", +] + +[[package]] +name = "aws-smithy-http" +version = "0.61.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6f276f21c7921fe902826618d1423ae5bf74cf8c1b8472aee8434f3dfd31824" dependencies = [ "aws-smithy-eventstream", "aws-smithy-runtime-api", @@ -535,7 +555,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d526a12d9ed61fadefda24abe2e682892ba288c2018bcb38b1b4c111d13f6d92" dependencies = [ "aws-smithy-async", - "aws-smithy-http", + "aws-smithy-http 0.60.12", "aws-smithy-runtime-api", "aws-smithy-types", "bytes", @@ -997,9 +1017,9 @@ checksum = "8f1fe948ff07f4bd06c30984e69f5b4899c516a3ef74f34df92a2df2ab535495" [[package]] name = "bytes" -version = "1.10.0" +version = "1.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f61dac84819c6588b558454b194026eb1f09c293b9036ae9b159e74e73ab6cf9" +checksum = "d71b6127be86fdcfddb610f7182ac57211d4b18a3e9c82eb2d17662f2227ad6a" [[package]] name = "bytes-utils" @@ -1730,9 +1750,9 @@ checksum = "1c7a8fb8a9fbf66c1f703fe16184d10ca0ee9d23be5b4436400408ba54a95005" [[package]] name = "either" -version = "1.14.0" +version = "1.15.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b7914353092ddf589ad78f25c5c1c21b7f80b0ff8621e7c814c3485b5306da9d" +checksum = "48c757948c5ede0e46177b7add2e67155f70e33c07fea8284df6576da70b3719" [[package]] name = "encode_unicode" @@ -2201,7 +2221,7 @@ dependencies = [ [[package]] name = "goose-bench" -version = "1.0.10" +version = "1.0.12" dependencies = [ "anyhow", "async-trait", @@ -4552,9 +4572,9 @@ dependencies = [ [[package]] name = "ring" -version = "0.17.11" +version = "0.17.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da5349ae27d3887ca812fb375b45a4fbb36d8d12d2df394968cd86e35683fe73" +checksum = "ed9b823fa29b721a59671b41d6b06e66b29e0628e207e8b1c3ceeda701ec928d" dependencies = [ "cc", "cfg-if", @@ -5475,9 +5495,9 @@ dependencies = [ [[package]] name = "time" -version = "0.3.37" +version = "0.3.38" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "35e7868883861bd0e56d9ac6efcaaca0d6d5d82a2a7ec8209ff492c07cf37b21" +checksum = "bb041120f25f8fbe8fd2dbe4671c7c2ed74d83be2e7a77529bf7e0790ae3f472" dependencies = [ "deranged", "itoa", @@ -5492,15 +5512,15 @@ dependencies = [ [[package]] name = "time-core" -version = "0.1.2" +version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ef927ca75afb808a4d64dd374f00a2adf8d0fcff8e7b184af886c3c87ec4a3f3" +checksum = "765c97a5b985b7c11d7bc27fa927dc4fe6af3a6dfb021d28deb60d3bf51e76ef" [[package]] name = "time-macros" -version = "0.2.19" +version = "0.2.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2834e6017e3e5e4b9834939793b282bc03b37a3336245fa820e35e233e2a85de" +checksum = "e8093bc3e81c3bc5f7879de09619d06c9a5a5e45ca44dfeeb7225bae38005c5c" dependencies = [ "num-conv", "time-core", diff --git a/crates/goose-bench/src/bench_work_dir.rs b/crates/goose-bench/src/bench_work_dir.rs new file mode 100644 index 000000000000..63203cd03a65 --- /dev/null +++ b/crates/goose-bench/src/bench_work_dir.rs @@ -0,0 +1,134 @@ +use chrono::Local; +use std::fs; +use std::io; +use std::path::Path; +use std::path::PathBuf; + +pub struct BenchmarkWorkDir { + pub base_path: PathBuf, + run_name: String, + suite: Option, + eval: Option, +} + +impl Default for BenchmarkWorkDir { + fn default() -> Self { + BenchmarkWorkDir::new("work_dir".to_string(), Vec::new()) + } +} +impl BenchmarkWorkDir { + pub fn new(work_dir_name: String, include_dirs: Vec) -> Self { + let base_path = PathBuf::from(format!("./benchmark-{}", work_dir_name)); + fs::create_dir_all(&base_path).unwrap(); + + let current_time = Local::now().format("T%H_%M_%S").to_string(); + let current_date = Local::now().format("%Y-%m-%d").to_string(); + let run_name = format!("{}-{}", ¤t_date, current_time); + + let mut base_path = PathBuf::from(&base_path).canonicalize().unwrap(); + base_path.push(run_name.clone()); + fs::create_dir_all(&base_path).unwrap(); + base_path.pop(); + + // abs paths from dir-strings + let dirs = include_dirs + .iter() + .map(|d| d.canonicalize().unwrap()) + .collect::>(); + + // deep copy each dir + let _: Vec<_> = dirs + .iter() + .map(|d| BenchmarkWorkDir::deep_copy(d.as_path(), base_path.as_path())) + .collect(); + + std::env::set_current_dir(&base_path).unwrap(); + + BenchmarkWorkDir { + base_path, + run_name, + suite: None, + eval: None, + } + } + pub fn cd(&mut self, path: PathBuf) -> anyhow::Result<&mut Self> { + fs::create_dir_all(&path)?; + std::env::set_current_dir(&path)?; + Ok(self) + } + pub fn set_suite(&mut self, suite: &str) { + self.eval = None; + self.suite = Some(suite.to_string()); + + let mut suite_dir = self.base_path.clone(); + suite_dir.push(self.run_name.clone()); + suite_dir.push(suite); + + self.cd(suite_dir.clone()) + .expect(format!("Failed to execute cd into {}", suite_dir.clone().display()).as_str()); + } + pub fn set_eval(&mut self, eval: &str) { + self.eval = Some(eval.to_string()); + + let mut eval_dir = self.base_path.clone(); + eval_dir.push(self.run_name.clone()); + eval_dir.push(self.suite.clone().unwrap()); + eval_dir.push(eval); + + self.cd(eval_dir.clone()) + .expect(format!("Failed to execute cd into {}", eval_dir.clone().display()).as_str()); + } + + pub fn fs_get(&mut self, path: String) -> anyhow::Result { + let p = Path::new(&path); + if !p.exists() { + let artifact_at_root = if p.is_dir() { + self.base_path.clone().join(&path).canonicalize()? + } else { + self.base_path + .clone() + .join(p.parent().unwrap_or(Path::new(""))) + .canonicalize()? + }; + + let here = PathBuf::from(".").canonicalize()?; + + BenchmarkWorkDir::deep_copy(artifact_at_root.as_path(), here.as_path())?; + } + + Ok(PathBuf::from(path)) + } + + fn deep_copy(src: &Path, dst: &Path) -> io::Result<()> { + // Create the destination directory with the source's name + let dst_dir = if let Some(src_name) = src.file_name() { + dst.join(src_name) + } else { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "Source path must have a file name", + )); + }; + + // Create the destination directory if it doesn't exist + if !dst_dir.exists() { + fs::create_dir_all(&dst_dir)?; + } + + // Copy each entry in the source directory + for entry in fs::read_dir(src)? { + let entry = entry?; + let ty = entry.file_type()?; + let src_path = entry.path(); + let dst_path = dst_dir.join(entry.file_name()); + + if ty.is_dir() { + BenchmarkWorkDir::deep_copy(&src_path, dst_path.parent().unwrap())?; + } else { + fs::copy(&src_path, &dst_path)?; + } + } + + Ok(()) + } +} diff --git a/crates/goose-bench/src/eval_suites/core/create_file.rs b/crates/goose-bench/src/eval_suites/core/create_file.rs index 9b6b285c08c8..ef920a4d0368 100644 --- a/crates/goose-bench/src/eval_suites/core/create_file.rs +++ b/crates/goose-bench/src/eval_suites/core/create_file.rs @@ -2,7 +2,7 @@ use crate::eval_suites::{BenchAgent, Evaluation, EvaluationMetric}; use crate::register_evaluation; -use crate::work_dir::WorkDir; +use crate::bench_work_dir::BenchmarkWorkDir; use async_trait::async_trait; use goose::message::MessageContent; use mcp_core::role::Role; @@ -22,7 +22,7 @@ impl Evaluation for DeveloperCreateFile { async fn run( &self, mut agent: Box, - _work_dir: &mut WorkDir, + _work_dir: &mut BenchmarkWorkDir, ) -> anyhow::Result> { let mut metrics = Vec::new(); diff --git a/crates/goose-bench/src/eval_suites/core/example.rs b/crates/goose-bench/src/eval_suites/core/example.rs index 7661232c416a..ac22acb11b33 100644 --- a/crates/goose-bench/src/eval_suites/core/example.rs +++ b/crates/goose-bench/src/eval_suites/core/example.rs @@ -1,6 +1,6 @@ use crate::eval_suites::{BenchAgent, Evaluation, EvaluationMetric}; use crate::register_evaluation; -use crate::work_dir::WorkDir; +use crate::bench_work_dir::BenchmarkWorkDir; use async_trait::async_trait; // use std::fs; @@ -17,7 +17,7 @@ impl Evaluation for ExampleEval { async fn run( &self, mut agent: Box, - _work_dir: &mut WorkDir, + _work_dir: &mut BenchmarkWorkDir, ) -> anyhow::Result> { println!("ExampleEval - run"); // let f = work_dir.fs_get(String::from("./arbitrary_dir/arbitrary_file.txt"))?; diff --git a/crates/goose-bench/src/eval_suites/core/image.rs b/crates/goose-bench/src/eval_suites/core/image.rs index 7b361350e593..0e8929efca2e 100644 --- a/crates/goose-bench/src/eval_suites/core/image.rs +++ b/crates/goose-bench/src/eval_suites/core/image.rs @@ -1,6 +1,6 @@ use crate::eval_suites::{BenchAgent, Evaluation, EvaluationMetric}; use crate::register_evaluation; -use crate::work_dir::WorkDir; +use crate::bench_work_dir::BenchmarkWorkDir; use async_trait::async_trait; use goose::message::MessageContent; use mcp_core::content::Content; @@ -21,7 +21,7 @@ impl Evaluation for DeveloperImage { async fn run( &self, mut agent: Box, - _work_dir: &mut WorkDir, + _work_dir: &mut BenchmarkWorkDir, ) -> anyhow::Result> { let mut metrics = Vec::new(); diff --git a/crates/goose-bench/src/eval_suites/core/list_files.rs b/crates/goose-bench/src/eval_suites/core/list_files.rs index 6af400444755..37ef55ad5a83 100644 --- a/crates/goose-bench/src/eval_suites/core/list_files.rs +++ b/crates/goose-bench/src/eval_suites/core/list_files.rs @@ -1,6 +1,6 @@ use crate::eval_suites::{BenchAgent, Evaluation, EvaluationMetric}; use crate::register_evaluation; -use crate::work_dir::WorkDir; +use crate::bench_work_dir::BenchmarkWorkDir; use async_trait::async_trait; use goose::message::MessageContent; use mcp_core::role::Role; @@ -20,7 +20,7 @@ impl Evaluation for DeveloperListFiles { async fn run( &self, mut agent: Box, - _work_dir: &mut WorkDir, + _work_dir: &mut BenchmarkWorkDir, ) -> anyhow::Result> { let mut metrics = Vec::new(); diff --git a/crates/goose-bench/src/eval_suites/core/save_fact.rs b/crates/goose-bench/src/eval_suites/core/save_fact.rs index 3051bdea1488..112e3314357b 100644 --- a/crates/goose-bench/src/eval_suites/core/save_fact.rs +++ b/crates/goose-bench/src/eval_suites/core/save_fact.rs @@ -2,7 +2,7 @@ use crate::eval_suites::{BenchAgent, Evaluation, EvaluationMetric}; use crate::register_evaluation; -use crate::work_dir::WorkDir; +use crate::bench_work_dir::BenchmarkWorkDir; use async_trait::async_trait; use goose::message::MessageContent; use mcp_core::role::Role; @@ -22,7 +22,7 @@ impl Evaluation for MemoryRememberMemory { async fn run( &self, mut agent: Box, - _work_dir: &mut WorkDir, + _work_dir: &mut BenchmarkWorkDir, ) -> anyhow::Result> { let mut metrics = Vec::new(); diff --git a/crates/goose-bench/src/eval_suites/core/script.rs b/crates/goose-bench/src/eval_suites/core/script.rs index 4a66a09c640b..ae3ad89fe02d 100644 --- a/crates/goose-bench/src/eval_suites/core/script.rs +++ b/crates/goose-bench/src/eval_suites/core/script.rs @@ -2,7 +2,7 @@ use crate::eval_suites::{BenchAgent, Evaluation, EvaluationMetric}; use crate::register_evaluation; -use crate::work_dir::WorkDir; +use crate::bench_work_dir::BenchmarkWorkDir; use async_trait::async_trait; use goose::message::MessageContent; use mcp_core::role::Role; @@ -22,7 +22,7 @@ impl Evaluation for ComputerControllerScript { async fn run( &self, mut agent: Box, - _work_dir: &mut WorkDir, + _work_dir: &mut BenchmarkWorkDir, ) -> anyhow::Result> { let mut metrics = Vec::new(); diff --git a/crates/goose-bench/src/eval_suites/core/search_replace.rs b/crates/goose-bench/src/eval_suites/core/search_replace.rs index 061cde024bcb..2f3565a17090 100644 --- a/crates/goose-bench/src/eval_suites/core/search_replace.rs +++ b/crates/goose-bench/src/eval_suites/core/search_replace.rs @@ -1,6 +1,6 @@ use crate::eval_suites::{BenchAgent, Evaluation, EvaluationMetric}; use crate::register_evaluation; -use crate::work_dir::WorkDir; +use crate::bench_work_dir::BenchmarkWorkDir; use async_trait::async_trait; use std::fs; @@ -18,17 +18,17 @@ impl Evaluation for DeveloperSearchReplace { async fn run( &self, mut agent: Box, - work_dir: &mut WorkDir, + work_dir: &mut BenchmarkWorkDir, ) -> anyhow::Result> { let mut metrics = Vec::new(); // Try to find the assets directory - let assets_dir_path = work_dir.path.join("assets"); + let assets_dir_path = work_dir.base_path.join("assets"); let _assets_exists = assets_dir_path.exists(); // Get the kubernetes_swagger.json file from the assets directory and copy it to the working directory for eval // so the agent can modify it - let source_file = work_dir.path.join("assets").join("kubernetes_swagger.json"); + let source_file = work_dir.base_path.join("assets").join("kubernetes_swagger.json"); let target_file = std::env::current_dir() .unwrap_or_default() .join("kubernetes_swagger.json"); @@ -53,7 +53,7 @@ impl Evaluation for DeveloperSearchReplace { .join("kubernetes_swagger.json"); // Read the expected patch file from the assets directory - let patch_file_path = work_dir.path.join("assets").join("kubernetes.patch"); + let patch_file_path = work_dir.base_path.join("assets").join("kubernetes.patch"); if !patch_file_path.exists() { return Err(anyhow::anyhow!("Could not find patch file")); } diff --git a/crates/goose-bench/src/eval_suites/core/web_scrape.rs b/crates/goose-bench/src/eval_suites/core/web_scrape.rs index 7b20850cd536..9d7622daebe3 100644 --- a/crates/goose-bench/src/eval_suites/core/web_scrape.rs +++ b/crates/goose-bench/src/eval_suites/core/web_scrape.rs @@ -2,7 +2,7 @@ use crate::eval_suites::{BenchAgent, Evaluation, EvaluationMetric}; use crate::register_evaluation; -use crate::work_dir::WorkDir; +use crate::bench_work_dir::BenchmarkWorkDir; use async_trait::async_trait; use goose::message::MessageContent; use mcp_core::role::Role; @@ -22,7 +22,7 @@ impl Evaluation for ComputerControllerWebScrape { async fn run( &self, mut agent: Box, - _work_dir: &mut WorkDir, + _work_dir: &mut BenchmarkWorkDir, ) -> anyhow::Result> { let mut metrics = Vec::new(); diff --git a/crates/goose-bench/src/eval_suites/evaluation.rs b/crates/goose-bench/src/eval_suites/evaluation.rs index 6e84916f5529..a4d5486b3733 100644 --- a/crates/goose-bench/src/eval_suites/evaluation.rs +++ b/crates/goose-bench/src/eval_suites/evaluation.rs @@ -1,4 +1,4 @@ -use crate::work_dir::WorkDir; +use crate::bench_work_dir::BenchmarkWorkDir; use anyhow::Result; use async_trait::async_trait; use chrono::{DateTime, Utc}; @@ -36,7 +36,7 @@ pub trait Evaluation: Send + Sync { async fn run( &self, agent: Box, - run_loc: &mut WorkDir, + run_loc: &mut BenchmarkWorkDir, ) -> Result>; fn name(&self) -> &str; diff --git a/crates/goose-bench/src/lib.rs b/crates/goose-bench/src/lib.rs index 4550c4cfb8db..5bc402c2129e 100644 --- a/crates/goose-bench/src/lib.rs +++ b/crates/goose-bench/src/lib.rs @@ -1,4 +1,4 @@ pub mod error_capture; pub mod eval_suites; pub mod reporting; -pub mod work_dir; +pub mod bench_work_dir; diff --git a/crates/goose-bench/src/work_dir.rs b/crates/goose-bench/src/work_dir.rs deleted file mode 100644 index f1a443cfe311..000000000000 --- a/crates/goose-bench/src/work_dir.rs +++ /dev/null @@ -1,113 +0,0 @@ -use std::fs; -use std::io; -use std::path::Path; -use std::path::PathBuf; - -pub struct WorkDir { - pub path: PathBuf, - traversal: Vec, -} - -impl Default for WorkDir { - fn default() -> Self { - let path = PathBuf::from(".").canonicalize().unwrap(); - WorkDir { - path: path.clone(), - traversal: vec![path.clone()], - } - } -} -impl WorkDir { - pub fn new(path: &str) -> Self { - let path = PathBuf::from(path); - WorkDir { - path: path.clone(), - traversal: vec![path.clone()], - } - } - - pub fn at(path: String, include_dirs: Vec) -> anyhow::Result { - fs::create_dir_all(&path)?; - - let dirs = include_dirs - .iter() - .map(|d| d.canonicalize().unwrap()) - .collect::>(); - - let p = PathBuf::from(&path).canonicalize()?; - let _: Vec<_> = dirs - .iter() - .map(|d| WorkDir::deep_copy(d.as_path(), p.as_path())) - .collect(); - - std::env::set_current_dir(&path)?; - - Ok(WorkDir::new(p.to_string_lossy().to_string().as_str())) - } - pub fn move_to(&mut self, path: String) -> anyhow::Result<&mut Self> { - fs::create_dir_all(&path)?; - self.traversal.push(PathBuf::from(&path)); - std::env::set_current_dir(&path)?; - Ok(self) - } - - pub fn fs_get(&mut self, path: String) -> anyhow::Result { - let p = Path::new(&path); - if !p.exists() { - let artifact_at_root = if p.is_dir() { - self.traversal[0].clone().join(&path).canonicalize()? - } else { - self.traversal[0] - .clone() - .join(p.parent().unwrap_or(Path::new(""))) - .canonicalize()? - }; - - let here = PathBuf::from(".").canonicalize()?; - - WorkDir::deep_copy(artifact_at_root.as_path(), here.as_path())?; - } - - Ok(PathBuf::from(path)) - } - - fn deep_copy(src: &Path, dst: &Path) -> io::Result<()> { - // Create the destination directory with the source's name - let dst_dir = if let Some(src_name) = src.file_name() { - dst.join(src_name) - } else { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - "Source path must have a file name", - )); - }; - - // Create the destination directory if it doesn't exist - if !dst_dir.exists() { - fs::create_dir_all(&dst_dir)?; - } - - // Copy each entry in the source directory - for entry in fs::read_dir(src)? { - let entry = entry?; - let ty = entry.file_type()?; - let src_path = entry.path(); - let dst_path = dst_dir.join(entry.file_name()); - - if ty.is_dir() { - WorkDir::deep_copy(&src_path, dst_path.parent().unwrap())?; - } else { - fs::copy(&src_path, &dst_path)?; - } - } - - Ok(()) - } -} - -impl Drop for WorkDir { - fn drop(&mut self) { - self.traversal.pop(); - std::env::set_current_dir("..").unwrap() - } -} diff --git a/crates/goose-cli/src/commands/bench.rs b/crates/goose-cli/src/commands/bench.rs index 823cd4ebc270..38a78babefcc 100644 --- a/crates/goose-cli/src/commands/bench.rs +++ b/crates/goose-cli/src/commands/bench.rs @@ -1,13 +1,12 @@ use crate::session::build_session; use crate::Session; use async_trait::async_trait; -use chrono::Local; use goose::config::Config; use goose::message::Message; +use goose_bench::bench_work_dir::BenchmarkWorkDir; use goose_bench::error_capture::ErrorCaptureLayer; use goose_bench::eval_suites::{BenchAgent, BenchAgentError, Evaluation, EvaluationSuiteFactory}; use goose_bench::reporting::{BenchmarkResults, EvaluationResult, SuiteResult}; -use goose_bench::work_dir::WorkDir; use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; @@ -77,47 +76,46 @@ impl BenchAgent for BenchAgentWrapper { async fn run_eval( evaluation: Box, - work_dir: &mut WorkDir, + work_dir: &mut BenchmarkWorkDir, ) -> anyhow::Result { let mut result = EvaluationResult::new(evaluation.name().to_string()); - if let Ok(work_dir) = work_dir.move_to(format!("./{}", &evaluation.name())) { - let required_extensions = evaluation.required_extensions(); + let required_extensions = evaluation.required_extensions(); - // Create session with error capture - let base_session = build_session(None, false, Vec::new(), required_extensions).await; + // Create session with error capture + let base_session = build_session(None, false, Vec::new(), required_extensions).await; - let bench_session = Arc::new(Mutex::new(BenchSession::new(base_session))); - let bench_session_clone = bench_session.clone(); + let bench_session = Arc::new(Mutex::new(BenchSession::new(base_session))); + let bench_session_clone = bench_session.clone(); - if let Ok(metrics) = evaluation - .run(Box::new(BenchAgentWrapper(bench_session)), work_dir) - .await - { - for (name, metric) in metrics { - result.add_metric(name, metric); - } - - // Add any errors that occurred - let agent = BenchAgentWrapper(bench_session_clone); - for error in agent.get_errors().await { - result.add_error(error); - } + if let Ok(metrics) = evaluation + .run(Box::new(BenchAgentWrapper(bench_session)), work_dir) + .await + { + for (name, metric) in metrics { + result.add_metric(name, metric); + } + + // Add any errors that occurred + let agent = BenchAgentWrapper(bench_session_clone); + for error in agent.get_errors().await { + result.add_error(error); } } Ok(result) } -async fn run_suite(suite: &str, work_dir: &mut WorkDir) -> anyhow::Result { +async fn run_suite(suite: &str, work_dir: &mut BenchmarkWorkDir) -> anyhow::Result { let mut suite_result = SuiteResult::new(suite.to_string()); - - if let Ok(work_dir) = work_dir.move_to(format!("./{}", &suite)) { - if let Some(evals) = EvaluationSuiteFactory::create(suite) { - for eval in evals { - let eval_result = run_eval(eval, work_dir).await?; - suite_result.add_evaluation(eval_result); - } + let eval_lock = Mutex::new(0); + + if let Some(evals) = EvaluationSuiteFactory::create(suite) { + for eval in evals { + let _unused = eval_lock.lock().await; + work_dir.set_eval(&eval.name()); + let eval_result = run_eval(eval, work_dir).await?; + suite_result.add_evaluation(eval_result); } } @@ -140,18 +138,13 @@ pub async fn run_benchmark( let mut results = BenchmarkResults::new(provider_name.clone()); - let current_time = Local::now().format("%H:%M:%S").to_string(); - let current_date = Local::now().format("%Y-%m-%d").to_string(); - if let Ok(mut work_dir) = WorkDir::at( - format!("./benchmark-{}", &provider_name), - include_dirs.clone(), - ) { - if let Ok(work_dir) = work_dir.move_to(format!("./{}-{}", ¤t_date, current_time)) { - for suite in suites { - let suite_result = run_suite(suite, work_dir).await?; - results.add_suite(suite_result); - } - } + let mut work_dir = BenchmarkWorkDir::new(provider_name, include_dirs.clone()); + let suite_lock = Mutex::new(0); + for suite in suites { + let _unused = suite_lock.lock().await; + work_dir.set_suite(&suite); + let suite_result = run_suite(suite, &mut work_dir).await?; + results.add_suite(suite_result); } Ok(results) From ed78d146d0a248177ac50f09422ab65627320e72 Mon Sep 17 00:00:00 2001 From: Marcelle Bonterre Date: Thu, 6 Mar 2025 13:53:32 -0500 Subject: [PATCH 2/8] fmt + clippy --- crates/goose-bench/src/bench_work_dir.rs | 7 ++++--- crates/goose-bench/src/eval_suites/core/create_file.rs | 2 +- crates/goose-bench/src/eval_suites/core/example.rs | 2 +- crates/goose-bench/src/eval_suites/core/image.rs | 2 +- crates/goose-bench/src/eval_suites/core/list_files.rs | 2 +- crates/goose-bench/src/eval_suites/core/save_fact.rs | 2 +- crates/goose-bench/src/eval_suites/core/script.rs | 2 +- crates/goose-bench/src/eval_suites/core/search_replace.rs | 7 +++++-- crates/goose-bench/src/eval_suites/core/web_scrape.rs | 2 +- crates/goose-bench/src/lib.rs | 2 +- crates/goose-cli/src/commands/bench.rs | 2 +- 11 files changed, 18 insertions(+), 14 deletions(-) diff --git a/crates/goose-bench/src/bench_work_dir.rs b/crates/goose-bench/src/bench_work_dir.rs index 63203cd03a65..be937deb9696 100644 --- a/crates/goose-bench/src/bench_work_dir.rs +++ b/crates/goose-bench/src/bench_work_dir.rs @@ -64,8 +64,9 @@ impl BenchmarkWorkDir { suite_dir.push(self.run_name.clone()); suite_dir.push(suite); - self.cd(suite_dir.clone()) - .expect(format!("Failed to execute cd into {}", suite_dir.clone().display()).as_str()); + self.cd(suite_dir.clone()).unwrap_or_else(|_| { + panic!("Failed to execute cd into {}", suite_dir.clone().display()) + }); } pub fn set_eval(&mut self, eval: &str) { self.eval = Some(eval.to_string()); @@ -76,7 +77,7 @@ impl BenchmarkWorkDir { eval_dir.push(eval); self.cd(eval_dir.clone()) - .expect(format!("Failed to execute cd into {}", eval_dir.clone().display()).as_str()); + .unwrap_or_else(|_| panic!("Failed to execute cd into {}", eval_dir.clone().display())); } pub fn fs_get(&mut self, path: String) -> anyhow::Result { diff --git a/crates/goose-bench/src/eval_suites/core/create_file.rs b/crates/goose-bench/src/eval_suites/core/create_file.rs index ef920a4d0368..aa6e8f3cfe39 100644 --- a/crates/goose-bench/src/eval_suites/core/create_file.rs +++ b/crates/goose-bench/src/eval_suites/core/create_file.rs @@ -1,8 +1,8 @@ // Create a new file called test.txt with the content 'Hello, World! +use crate::bench_work_dir::BenchmarkWorkDir; use crate::eval_suites::{BenchAgent, Evaluation, EvaluationMetric}; use crate::register_evaluation; -use crate::bench_work_dir::BenchmarkWorkDir; use async_trait::async_trait; use goose::message::MessageContent; use mcp_core::role::Role; diff --git a/crates/goose-bench/src/eval_suites/core/example.rs b/crates/goose-bench/src/eval_suites/core/example.rs index ac22acb11b33..ad30830573a2 100644 --- a/crates/goose-bench/src/eval_suites/core/example.rs +++ b/crates/goose-bench/src/eval_suites/core/example.rs @@ -1,6 +1,6 @@ +use crate::bench_work_dir::BenchmarkWorkDir; use crate::eval_suites::{BenchAgent, Evaluation, EvaluationMetric}; use crate::register_evaluation; -use crate::bench_work_dir::BenchmarkWorkDir; use async_trait::async_trait; // use std::fs; diff --git a/crates/goose-bench/src/eval_suites/core/image.rs b/crates/goose-bench/src/eval_suites/core/image.rs index 0e8929efca2e..9594c94f4beb 100644 --- a/crates/goose-bench/src/eval_suites/core/image.rs +++ b/crates/goose-bench/src/eval_suites/core/image.rs @@ -1,6 +1,6 @@ +use crate::bench_work_dir::BenchmarkWorkDir; use crate::eval_suites::{BenchAgent, Evaluation, EvaluationMetric}; use crate::register_evaluation; -use crate::bench_work_dir::BenchmarkWorkDir; use async_trait::async_trait; use goose::message::MessageContent; use mcp_core::content::Content; diff --git a/crates/goose-bench/src/eval_suites/core/list_files.rs b/crates/goose-bench/src/eval_suites/core/list_files.rs index 37ef55ad5a83..9cedf88b8c69 100644 --- a/crates/goose-bench/src/eval_suites/core/list_files.rs +++ b/crates/goose-bench/src/eval_suites/core/list_files.rs @@ -1,6 +1,6 @@ +use crate::bench_work_dir::BenchmarkWorkDir; use crate::eval_suites::{BenchAgent, Evaluation, EvaluationMetric}; use crate::register_evaluation; -use crate::bench_work_dir::BenchmarkWorkDir; use async_trait::async_trait; use goose::message::MessageContent; use mcp_core::role::Role; diff --git a/crates/goose-bench/src/eval_suites/core/save_fact.rs b/crates/goose-bench/src/eval_suites/core/save_fact.rs index 112e3314357b..c361fc50500c 100644 --- a/crates/goose-bench/src/eval_suites/core/save_fact.rs +++ b/crates/goose-bench/src/eval_suites/core/save_fact.rs @@ -1,8 +1,8 @@ // Create a new file called test.txt with the content 'Hello, World! +use crate::bench_work_dir::BenchmarkWorkDir; use crate::eval_suites::{BenchAgent, Evaluation, EvaluationMetric}; use crate::register_evaluation; -use crate::bench_work_dir::BenchmarkWorkDir; use async_trait::async_trait; use goose::message::MessageContent; use mcp_core::role::Role; diff --git a/crates/goose-bench/src/eval_suites/core/script.rs b/crates/goose-bench/src/eval_suites/core/script.rs index ae3ad89fe02d..8ee34e9fa1c5 100644 --- a/crates/goose-bench/src/eval_suites/core/script.rs +++ b/crates/goose-bench/src/eval_suites/core/script.rs @@ -1,8 +1,8 @@ // Create a new file called test.txt with the content 'Hello, World! +use crate::bench_work_dir::BenchmarkWorkDir; use crate::eval_suites::{BenchAgent, Evaluation, EvaluationMetric}; use crate::register_evaluation; -use crate::bench_work_dir::BenchmarkWorkDir; use async_trait::async_trait; use goose::message::MessageContent; use mcp_core::role::Role; diff --git a/crates/goose-bench/src/eval_suites/core/search_replace.rs b/crates/goose-bench/src/eval_suites/core/search_replace.rs index 2f3565a17090..9bf8ebb03cd4 100644 --- a/crates/goose-bench/src/eval_suites/core/search_replace.rs +++ b/crates/goose-bench/src/eval_suites/core/search_replace.rs @@ -1,6 +1,6 @@ +use crate::bench_work_dir::BenchmarkWorkDir; use crate::eval_suites::{BenchAgent, Evaluation, EvaluationMetric}; use crate::register_evaluation; -use crate::bench_work_dir::BenchmarkWorkDir; use async_trait::async_trait; use std::fs; @@ -28,7 +28,10 @@ impl Evaluation for DeveloperSearchReplace { // Get the kubernetes_swagger.json file from the assets directory and copy it to the working directory for eval // so the agent can modify it - let source_file = work_dir.base_path.join("assets").join("kubernetes_swagger.json"); + let source_file = work_dir + .base_path + .join("assets") + .join("kubernetes_swagger.json"); let target_file = std::env::current_dir() .unwrap_or_default() .join("kubernetes_swagger.json"); diff --git a/crates/goose-bench/src/eval_suites/core/web_scrape.rs b/crates/goose-bench/src/eval_suites/core/web_scrape.rs index 9d7622daebe3..86db10564555 100644 --- a/crates/goose-bench/src/eval_suites/core/web_scrape.rs +++ b/crates/goose-bench/src/eval_suites/core/web_scrape.rs @@ -1,8 +1,8 @@ // Create a new file called test.txt with the content 'Hello, World! +use crate::bench_work_dir::BenchmarkWorkDir; use crate::eval_suites::{BenchAgent, Evaluation, EvaluationMetric}; use crate::register_evaluation; -use crate::bench_work_dir::BenchmarkWorkDir; use async_trait::async_trait; use goose::message::MessageContent; use mcp_core::role::Role; diff --git a/crates/goose-bench/src/lib.rs b/crates/goose-bench/src/lib.rs index 5bc402c2129e..fd65b414376c 100644 --- a/crates/goose-bench/src/lib.rs +++ b/crates/goose-bench/src/lib.rs @@ -1,4 +1,4 @@ +pub mod bench_work_dir; pub mod error_capture; pub mod eval_suites; pub mod reporting; -pub mod bench_work_dir; diff --git a/crates/goose-cli/src/commands/bench.rs b/crates/goose-cli/src/commands/bench.rs index 38a78babefcc..d5653b02457e 100644 --- a/crates/goose-cli/src/commands/bench.rs +++ b/crates/goose-cli/src/commands/bench.rs @@ -142,7 +142,7 @@ pub async fn run_benchmark( let suite_lock = Mutex::new(0); for suite in suites { let _unused = suite_lock.lock().await; - work_dir.set_suite(&suite); + work_dir.set_suite(suite); let suite_result = run_suite(suite, &mut work_dir).await?; results.add_suite(suite_result); } From 370646de3cc6934f52cc1995483f164238f4e3af Mon Sep 17 00:00:00 2001 From: Marcelle Bonterre Date: Thu, 6 Mar 2025 14:02:37 -0500 Subject: [PATCH 3/8] clippy --- crates/goose-cli/src/commands/bench.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/goose-cli/src/commands/bench.rs b/crates/goose-cli/src/commands/bench.rs index d5653b02457e..f5e7b660287d 100644 --- a/crates/goose-cli/src/commands/bench.rs +++ b/crates/goose-cli/src/commands/bench.rs @@ -113,7 +113,7 @@ async fn run_suite(suite: &str, work_dir: &mut BenchmarkWorkDir) -> anyhow::Resu if let Some(evals) = EvaluationSuiteFactory::create(suite) { for eval in evals { let _unused = eval_lock.lock().await; - work_dir.set_eval(&eval.name()); + work_dir.set_eval(eval.name()); let eval_result = run_eval(eval, work_dir).await?; suite_result.add_evaluation(eval_result); } From 093a6c16973b54453cbd6708f24e0c505dc5c220 Mon Sep 17 00:00:00 2001 From: Marcelle Bonterre Date: Thu, 6 Mar 2025 17:21:59 -0500 Subject: [PATCH 4/8] update deep copy func --- crates/goose-bench/src/bench_work_dir.rs | 123 +++++++++++------- .../src/eval_suites/core/search_replace.rs | 32 ++--- 2 files changed, 87 insertions(+), 68 deletions(-) diff --git a/crates/goose-bench/src/bench_work_dir.rs b/crates/goose-bench/src/bench_work_dir.rs index be937deb9696..534ef433feea 100644 --- a/crates/goose-bench/src/bench_work_dir.rs +++ b/crates/goose-bench/src/bench_work_dir.rs @@ -1,11 +1,14 @@ use chrono::Local; use std::fs; use std::io; +use std::io::ErrorKind; use std::path::Path; use std::path::PathBuf; +use std::process::Command; pub struct BenchmarkWorkDir { pub base_path: PathBuf, + cwd: PathBuf, run_name: String, suite: Option, eval: Option, @@ -39,13 +42,14 @@ impl BenchmarkWorkDir { // deep copy each dir let _: Vec<_> = dirs .iter() - .map(|d| BenchmarkWorkDir::deep_copy(d.as_path(), base_path.as_path())) + .map(|d| BenchmarkWorkDir::cp(d.as_path(), base_path.as_path(), true)) .collect(); std::env::set_current_dir(&base_path).unwrap(); BenchmarkWorkDir { - base_path, + base_path: base_path.clone(), + cwd: base_path.clone(), run_name, suite: None, eval: None, @@ -54,6 +58,7 @@ impl BenchmarkWorkDir { pub fn cd(&mut self, path: PathBuf) -> anyhow::Result<&mut Self> { fs::create_dir_all(&path)?; std::env::set_current_dir(&path)?; + self.cwd = path; Ok(self) } pub fn set_suite(&mut self, suite: &str) { @@ -80,56 +85,86 @@ impl BenchmarkWorkDir { .unwrap_or_else(|_| panic!("Failed to execute cd into {}", eval_dir.clone().display())); } + + fn chop_relative_base>(path: P) -> anyhow::Result { + let path = path.as_ref(); + + // Get the path components as an iterator + let mut components = path.components(); + + // Check the first component + if let Some(first) = components.next() { + use std::path::Component; + + match first { + Component::ParentDir => Err(anyhow::anyhow!("RelativePathBaseError: Only paths relative to the current working directory are supported.")), + // If first component is "." + Component::CurDir => Ok(components.collect()), + // Otherwise, keep the full path + _ => { + // Create a new PathBuf + let mut result = PathBuf::new(); + // Add back the first component + result.push(first); + // Add all remaining components + result.extend(components); + Ok(result) + } + } + } else { + // Empty path + Ok(PathBuf::new()) + } + } + + pub fn fs_get(&mut self, path: String) -> anyhow::Result { - let p = Path::new(&path); - if !p.exists() { - let artifact_at_root = if p.is_dir() { - self.base_path.clone().join(&path).canonicalize()? - } else { - self.base_path - .clone() - .join(p.parent().unwrap_or(Path::new(""))) - .canonicalize()? - }; - - let here = PathBuf::from(".").canonicalize()?; - - BenchmarkWorkDir::deep_copy(artifact_at_root.as_path(), here.as_path())?; + let p = PathBuf::from(&path); + if p.exists() { + return Ok(PathBuf::from(path)); } + if p.is_absolute() { + return Err(anyhow::anyhow!("AbsolutePathError: Only paths relative to the current working directory are supported.")); + } + + let asset_rel_path = Self::chop_relative_base(p.clone()) + .unwrap_or_else(|_| panic!("AbsolutePathError: Only paths relative to the current working directory are supported.")); + + let here = PathBuf::from(".").canonicalize()?; + let artifact_at_root = self.base_path.clone().join(asset_rel_path); + + BenchmarkWorkDir::cp(artifact_at_root.as_path(), here.as_path(), true)?; Ok(PathBuf::from(path)) } - fn deep_copy(src: &Path, dst: &Path) -> io::Result<()> { - // Create the destination directory with the source's name - let dst_dir = if let Some(src_name) = src.file_name() { - dst.join(src_name) - } else { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - "Source path must have a file name", - )); - }; - - // Create the destination directory if it doesn't exist - if !dst_dir.exists() { - fs::create_dir_all(&dst_dir)?; - } + fn cp(src: P, dst: Q, recursive: bool) -> io::Result<()> + where + P: AsRef, + Q: AsRef, + { + let src = src.as_ref(); + let dst = dst.as_ref(); - // Copy each entry in the source directory - for entry in fs::read_dir(src)? { - let entry = entry?; - let ty = entry.file_type()?; - let src_path = entry.path(); - let dst_path = dst_dir.join(entry.file_name()); - - if ty.is_dir() { - BenchmarkWorkDir::deep_copy(&src_path, dst_path.parent().unwrap())?; - } else { - fs::copy(&src_path, &dst_path)?; - } + let mut cmd = Command::new("cp"); + + // Add -r flag if recursive is true + if recursive { + cmd.arg("-r"); } - Ok(()) + // Add source and destination paths + cmd.arg(src).arg(dst); + + // Execute the command + let output = cmd.output()?; + + if output.status.success() { + Ok(()) + } else { + let error_message = String::from_utf8_lossy(&output.stderr).to_string(); + Err(io::Error::new(ErrorKind::Other, error_message)) + } } + } diff --git a/crates/goose-bench/src/eval_suites/core/search_replace.rs b/crates/goose-bench/src/eval_suites/core/search_replace.rs index 9bf8ebb03cd4..16d8c223c6bf 100644 --- a/crates/goose-bench/src/eval_suites/core/search_replace.rs +++ b/crates/goose-bench/src/eval_suites/core/search_replace.rs @@ -22,30 +22,14 @@ impl Evaluation for DeveloperSearchReplace { ) -> anyhow::Result> { let mut metrics = Vec::new(); - // Try to find the assets directory - let assets_dir_path = work_dir.base_path.join("assets"); - let _assets_exists = assets_dir_path.exists(); - - // Get the kubernetes_swagger.json file from the assets directory and copy it to the working directory for eval - // so the agent can modify it - let source_file = work_dir - .base_path - .join("assets") - .join("kubernetes_swagger.json"); - let target_file = std::env::current_dir() - .unwrap_or_default() - .join("kubernetes_swagger.json"); - - // Copy the file to the root of the working directory if it doesn't exist there yet - if !target_file.exists() && source_file.exists() { - println!("Copying file from {:?} to {:?}", source_file, target_file); - fs::copy(&source_file, &target_file)?; - println!("File copied successfully"); - } else { - return Err(anyhow::anyhow!( - "Could not find kubernetes_swagger.json file" - )); - } + let _target_file = match work_dir.fs_get("./assets/kubernetes_swagger.json".to_string()) { + Ok(file) => file, + Err(e) => { + return Err(anyhow::anyhow!("Could not find kubernetes_swagger.json file")) + } + }; + let mut source_file = work_dir.base_path.clone(); + source_file.push("assets/kubernetes_swagger.json"); // Send the prompt to modify the file let _messages = agent.prompt("Remove the io.k8s.api.admissionregistration.v1.ServiceReference definition block and replace with a new definition for io.k8s.api.admissionregistration.v1.FakeServiceReference. Update the fields in the definition as well to be consistent. Don't change the property names. Don't update any references to the old definition. Only modify the definition and it's description to 'FakeServiceReference simulates a reference to a fake service for testing purposes.'.The file to modify is kubernetes_swagger.json.".to_string()).await?; From cc046c147e2433f108ce13458715b259002809bd Mon Sep 17 00:00:00 2001 From: Marcelle Bonterre Date: Thu, 6 Mar 2025 17:22:21 -0500 Subject: [PATCH 5/8] update deep copy func --- crates/goose-bench/src/bench_work_dir.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/goose-bench/src/bench_work_dir.rs b/crates/goose-bench/src/bench_work_dir.rs index 534ef433feea..ebc09f178f7e 100644 --- a/crates/goose-bench/src/bench_work_dir.rs +++ b/crates/goose-bench/src/bench_work_dir.rs @@ -42,7 +42,7 @@ impl BenchmarkWorkDir { // deep copy each dir let _: Vec<_> = dirs .iter() - .map(|d| BenchmarkWorkDir::cp(d.as_path(), base_path.as_path(), true)) + .map(|d| BenchmarkWorkDir::deep_copy(d.as_path(), base_path.as_path(), true)) .collect(); std::env::set_current_dir(&base_path).unwrap(); @@ -134,11 +134,11 @@ impl BenchmarkWorkDir { let here = PathBuf::from(".").canonicalize()?; let artifact_at_root = self.base_path.clone().join(asset_rel_path); - BenchmarkWorkDir::cp(artifact_at_root.as_path(), here.as_path(), true)?; + BenchmarkWorkDir::deep_copy(artifact_at_root.as_path(), here.as_path(), true)?; Ok(PathBuf::from(path)) } - fn cp(src: P, dst: Q, recursive: bool) -> io::Result<()> + fn deep_copy(src: P, dst: Q, recursive: bool) -> io::Result<()> where P: AsRef, Q: AsRef, From b8905d9a8516be09ebdae2144ffb3c05558a8baf Mon Sep 17 00:00:00 2001 From: Marcelle Bonterre Date: Thu, 6 Mar 2025 17:24:09 -0500 Subject: [PATCH 6/8] fmt --- crates/goose-bench/src/bench_work_dir.rs | 3 --- crates/goose-bench/src/eval_suites/core/search_replace.rs | 6 ++++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/goose-bench/src/bench_work_dir.rs b/crates/goose-bench/src/bench_work_dir.rs index ebc09f178f7e..f9b25ae1c4c2 100644 --- a/crates/goose-bench/src/bench_work_dir.rs +++ b/crates/goose-bench/src/bench_work_dir.rs @@ -85,7 +85,6 @@ impl BenchmarkWorkDir { .unwrap_or_else(|_| panic!("Failed to execute cd into {}", eval_dir.clone().display())); } - fn chop_relative_base>(path: P) -> anyhow::Result { let path = path.as_ref(); @@ -117,7 +116,6 @@ impl BenchmarkWorkDir { } } - pub fn fs_get(&mut self, path: String) -> anyhow::Result { let p = PathBuf::from(&path); if p.exists() { @@ -166,5 +164,4 @@ impl BenchmarkWorkDir { Err(io::Error::new(ErrorKind::Other, error_message)) } } - } diff --git a/crates/goose-bench/src/eval_suites/core/search_replace.rs b/crates/goose-bench/src/eval_suites/core/search_replace.rs index 16d8c223c6bf..6ac654e206ca 100644 --- a/crates/goose-bench/src/eval_suites/core/search_replace.rs +++ b/crates/goose-bench/src/eval_suites/core/search_replace.rs @@ -24,8 +24,10 @@ impl Evaluation for DeveloperSearchReplace { let _target_file = match work_dir.fs_get("./assets/kubernetes_swagger.json".to_string()) { Ok(file) => file, - Err(e) => { - return Err(anyhow::anyhow!("Could not find kubernetes_swagger.json file")) + Err(_) => { + return Err(anyhow::anyhow!( + "Could not find kubernetes_swagger.json file" + )) } }; let mut source_file = work_dir.base_path.clone(); From 3e0913fb6a1061fbfa33ce3c5bb179ce5f8273cc Mon Sep 17 00:00:00 2001 From: Marcelle Bonterre Date: Thu, 6 Mar 2025 18:25:48 -0500 Subject: [PATCH 7/8] revert time fmt + capture model --- crates/goose-bench/src/bench_work_dir.rs | 2 +- crates/goose-cli/src/commands/bench.rs | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/goose-bench/src/bench_work_dir.rs b/crates/goose-bench/src/bench_work_dir.rs index f9b25ae1c4c2..941fe75b3e39 100644 --- a/crates/goose-bench/src/bench_work_dir.rs +++ b/crates/goose-bench/src/bench_work_dir.rs @@ -24,7 +24,7 @@ impl BenchmarkWorkDir { let base_path = PathBuf::from(format!("./benchmark-{}", work_dir_name)); fs::create_dir_all(&base_path).unwrap(); - let current_time = Local::now().format("T%H_%M_%S").to_string(); + let current_time = Local::now().format("%H:%M:%S").to_string(); let current_date = Local::now().format("%Y-%m-%d").to_string(); let run_name = format!("{}-{}", ¤t_date, current_time); diff --git a/crates/goose-cli/src/commands/bench.rs b/crates/goose-cli/src/commands/bench.rs index f5e7b660287d..ac534409e161 100644 --- a/crates/goose-cli/src/commands/bench.rs +++ b/crates/goose-cli/src/commands/bench.rs @@ -132,13 +132,19 @@ pub async fn run_benchmark( .collect::>(); let config = Config::global(); + let goose_model: String = config + .get("GOOSE_MODEL") + .expect("No model configured. Run 'goose configure' first"); let provider_name: String = config .get("GOOSE_PROVIDER") .expect("No provider configured. Run 'goose configure' first"); let mut results = BenchmarkResults::new(provider_name.clone()); - let mut work_dir = BenchmarkWorkDir::new(provider_name, include_dirs.clone()); + let mut work_dir = BenchmarkWorkDir::new( + format!("{}-{}", provider_name, goose_model), + include_dirs.clone(), + ); let suite_lock = Mutex::new(0); for suite in suites { let _unused = suite_lock.lock().await; From 84a4d403b6737f81f84ca9197ce633b118c78478 Mon Sep 17 00:00:00 2001 From: Marcelle Bonterre Date: Thu, 6 Mar 2025 21:00:51 -0500 Subject: [PATCH 8/8] last merge nit --- crates/goose-cli/src/commands/bench.rs | 38 ++++++++++++-------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/crates/goose-cli/src/commands/bench.rs b/crates/goose-cli/src/commands/bench.rs index 30bb42a73a1b..e83f146062d1 100644 --- a/crates/goose-cli/src/commands/bench.rs +++ b/crates/goose-cli/src/commands/bench.rs @@ -80,29 +80,27 @@ async fn run_eval( ) -> anyhow::Result { let mut result = EvaluationResult::new(evaluation.name().to_string()); - if let Ok(work_dir) = work_dir.move_to(format!("./{}", &evaluation.name())) { - let requirements = evaluation.required_extensions(); + let requirements = evaluation.required_extensions(); - // Create session with error capture - let base_session = - build_session(None, false, requirements.external, requirements.builtin).await; + // Create session with error capture + let base_session = + build_session(None, false, requirements.external, requirements.builtin).await; - let bench_session = Arc::new(Mutex::new(BenchSession::new(base_session))); - let bench_session_clone = bench_session.clone(); + let bench_session = Arc::new(Mutex::new(BenchSession::new(base_session))); + let bench_session_clone = bench_session.clone(); - if let Ok(metrics) = evaluation - .run(Box::new(BenchAgentWrapper(bench_session)), work_dir) - .await - { - for (name, metric) in metrics { - result.add_metric(name, metric); - } - - // Add any errors that occurred - let agent = BenchAgentWrapper(bench_session_clone); - for error in agent.get_errors().await { - result.add_error(error); - } + if let Ok(metrics) = evaluation + .run(Box::new(BenchAgentWrapper(bench_session)), work_dir) + .await + { + for (name, metric) in metrics { + result.add_metric(name, metric); + } + + // Add any errors that occurred + let agent = BenchAgentWrapper(bench_session_clone); + for error in agent.get_errors().await { + result.add_error(error); } }