From 60e8c561354f410c23b34d82a34806332100155d Mon Sep 17 00:00:00 2001 From: hanafish <1106510024@qq.com> Date: Fri, 7 Aug 2026 22:32:47 +0800 Subject: [PATCH] fix(benchmark): remove personal SWE-bench path defaults --- src-tauri/src/benchmark/mod.rs | 1 - src-tauri/src/benchmark/paths.rs | 121 ++++++++++++++++++++++++--- src-tauri/src/benchmark/preflight.rs | 18 ++-- src-tauri/src/benchmark/swe_bench.rs | 33 ++++---- 4 files changed, 143 insertions(+), 30 deletions(-) diff --git a/src-tauri/src/benchmark/mod.rs b/src-tauri/src/benchmark/mod.rs index 485adf0f14..4419c4fa57 100644 --- a/src-tauri/src/benchmark/mod.rs +++ b/src-tauri/src/benchmark/mod.rs @@ -57,7 +57,6 @@ const BENCHMARK_BATCH_TASK_ACTION_CANCEL: &str = "cancel"; const BENCHMARK_BATCH_TASK_ACTION_RESTART: &str = "restart"; const DEFAULT_AGENT_BATCH_CONCURRENCY: usize = 2; const MAX_AGENT_BATCH_CONCURRENCY: usize = 8; -const SWE_BENCH_PRO_REPO_PATH: &str = "/Users/laptop-h/Documents/GitHub/SWE-bench_Pro-os"; const SWE_BENCH_PRO_EVALUATOR_SCRIPT: &str = "swe_bench_pro_eval.py"; const SWE_BENCH_PRO_RUN_SCRIPTS_DIR: &str = "run_scripts"; const SWE_BENCH_PRO_DOCKERHUB_USERNAME: &str = "jefzda"; diff --git a/src-tauri/src/benchmark/paths.rs b/src-tauri/src/benchmark/paths.rs index f4ca0b136d..6d3cad673e 100644 --- a/src-tauri/src/benchmark/paths.rs +++ b/src-tauri/src/benchmark/paths.rs @@ -4,25 +4,63 @@ use std::path::PathBuf; use super::{ BENCHMARK_AGENT_SUBMISSIONS_DIR, BENCHMARK_AGENT_SUBMISSION_PATCH_FILE, - SWE_BENCH_PRO_EVALUATOR_SCRIPT, SWE_BENCH_PRO_REPO_PATH, SWE_BENCH_PRO_RUN_SCRIPTS_DIR, + SWE_BENCH_PRO_EVALUATOR_SCRIPT, SWE_BENCH_PRO_RUN_SCRIPTS_DIR, }; -pub(super) fn modal_config_path() -> PathBuf { - app_paths::home_dir().join(".modal.toml") +/// Environment variable naming the local SWE-bench Pro harness checkout +/// (the repo containing the evaluator script and per-task run scripts). +pub(super) const SWE_BENCH_PRO_REPO_PATH_ENV: &str = "ORGII_SWE_BENCH_PRO_REPO_PATH"; + +/// Typed "not configured" state for the SWE-bench Pro harness repository. +/// +/// There is intentionally no filesystem default: callers must surface this as +/// a configure-first error instead of failing later on a missing file. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) struct SweBenchRepoNotConfigured; + +impl SweBenchRepoNotConfigured { + pub(super) fn message(self) -> String { + format!( + "SWE-bench Pro repository path is not configured. Set the {SWE_BENCH_PRO_REPO_PATH_ENV} environment variable to a local SWE-bench Pro checkout before running Docker evaluation." + ) + } } -fn swe_bench_repo_path() -> PathBuf { - std::env::var("ORGII_SWE_BENCH_PRO_REPO_PATH") +/// Resolves the harness repo path with explicit-first precedence: +/// explicit value (app setting / command argument) → environment variable → +/// typed "not configured" error. Blank values count as unset. +fn resolve_swe_bench_repo_path( + explicit: Option<&str>, + env_value: Option<&str>, +) -> Result { + [explicit, env_value] + .into_iter() + .flatten() + .map(str::trim) + .find(|value| !value.is_empty()) .map(PathBuf::from) - .unwrap_or_else(|_| PathBuf::from(SWE_BENCH_PRO_REPO_PATH)) + .ok_or(SweBenchRepoNotConfigured) +} + +fn swe_bench_repo_path() -> Result { + // No explicit app setting exists yet, so the environment variable is the + // only configuration source (also used by the E2E suite fixture). + resolve_swe_bench_repo_path( + None, + std::env::var(SWE_BENCH_PRO_REPO_PATH_ENV).ok().as_deref(), + ) } -pub(super) fn swe_bench_evaluator_script_path() -> PathBuf { - swe_bench_repo_path().join(SWE_BENCH_PRO_EVALUATOR_SCRIPT) +pub(super) fn modal_config_path() -> PathBuf { + app_paths::home_dir().join(".modal.toml") +} + +pub(super) fn swe_bench_evaluator_script_path() -> Result { + Ok(swe_bench_repo_path()?.join(SWE_BENCH_PRO_EVALUATOR_SCRIPT)) } -pub(super) fn swe_bench_run_scripts_dir() -> PathBuf { - swe_bench_repo_path().join(SWE_BENCH_PRO_RUN_SCRIPTS_DIR) +pub(super) fn swe_bench_run_scripts_dir() -> Result { + Ok(swe_bench_repo_path()?.join(SWE_BENCH_PRO_RUN_SCRIPTS_DIR)) } fn benchmark_runs_dir() -> PathBuf { @@ -64,3 +102,66 @@ pub(super) fn benchmark_python_path() -> PathBuf { pub(super) fn benchmark_run_output_dir(run_id: &str) -> PathBuf { benchmark_runs_dir().join(run_id) } + +#[cfg(test)] +mod tests { + use super::{ + resolve_swe_bench_repo_path, SweBenchRepoNotConfigured, SWE_BENCH_PRO_REPO_PATH_ENV, + }; + use std::path::PathBuf; + + #[test] + fn explicit_value_wins_over_environment() { + assert_eq!( + resolve_swe_bench_repo_path(Some("/explicit/harness"), Some("/env/harness")), + Ok(PathBuf::from("/explicit/harness")) + ); + } + + #[test] + fn environment_is_used_when_no_explicit_value() { + assert_eq!( + resolve_swe_bench_repo_path(None, Some("/env/harness")), + Ok(PathBuf::from("/env/harness")) + ); + } + + #[test] + fn blank_explicit_value_falls_back_to_environment() { + assert_eq!( + resolve_swe_bench_repo_path(Some(" "), Some("/env/harness")), + Ok(PathBuf::from("/env/harness")) + ); + } + + #[test] + fn resolved_paths_are_trimmed() { + assert_eq!( + resolve_swe_bench_repo_path(None, Some(" /env/harness ")), + Ok(PathBuf::from("/env/harness")) + ); + } + + #[test] + fn missing_configuration_is_a_typed_error() { + assert_eq!( + resolve_swe_bench_repo_path(None, None), + Err(SweBenchRepoNotConfigured) + ); + } + + #[test] + fn blank_configuration_counts_as_unset() { + assert_eq!( + resolve_swe_bench_repo_path(Some(""), Some(" ")), + Err(SweBenchRepoNotConfigured) + ); + } + + #[test] + fn not_configured_message_names_the_environment_variable() { + assert!(SweBenchRepoNotConfigured + .message() + .contains(SWE_BENCH_PRO_REPO_PATH_ENV)); + } +} diff --git a/src-tauri/src/benchmark/preflight.rs b/src-tauri/src/benchmark/preflight.rs index fcc04db2c2..dacc1e8d74 100644 --- a/src-tauri/src/benchmark/preflight.rs +++ b/src-tauri/src/benchmark/preflight.rs @@ -240,19 +240,27 @@ async fn push_swe_bench_local_docker_checks( checks.push(BenchmarkPreflightCheck { id: "evaluator_script".to_string(), label: "SWE-bench Pro evaluator script".to_string(), - ok: evaluator_script.is_file(), - detail: Some(evaluator_script.display().to_string()), + ok: evaluator_script + .as_ref() + .is_ok_and(|script| script.is_file()), + detail: Some(match &evaluator_script { + Ok(script) => script.display().to_string(), + Err(error) => error.message(), + }), }); let scripts_dir = swe_bench_run_scripts_dir(); checks.push(BenchmarkPreflightCheck { id: "run_scripts_dir".to_string(), label: "SWE-bench Pro run scripts".to_string(), - ok: scripts_dir.is_dir(), - detail: Some(scripts_dir.display().to_string()), + ok: scripts_dir.as_ref().is_ok_and(|dir| dir.is_dir()), + detail: Some(match &scripts_dir { + Ok(dir) => dir.display().to_string(), + Err(error) => error.message(), + }), }); - if let Some(selected_task_id) = task_id { + if let (Some(selected_task_id), Ok(scripts_dir)) = (task_id, scripts_dir) { let run_script = scripts_dir.join(selected_task_id).join("run_script.sh"); checks.push(BenchmarkPreflightCheck { id: "task_run_script".to_string(), diff --git a/src-tauri/src/benchmark/swe_bench.rs b/src-tauri/src/benchmark/swe_bench.rs index 0401b353eb..0dfb357b06 100644 --- a/src-tauri/src/benchmark/swe_bench.rs +++ b/src-tauri/src/benchmark/swe_bench.rs @@ -76,25 +76,28 @@ pub(super) async fn build_swe_bench_run_plan( patch_path }; - let evaluator_script = if evaluation_mode == EVALUATION_MODE_LOCAL_DOCKER { - Some(swe_bench_evaluator_script_path()) + let (evaluator_script, scripts_dir) = if evaluation_mode == EVALUATION_MODE_LOCAL_DOCKER { + let evaluator_script = + swe_bench_evaluator_script_path().map_err(|error| error.message())?; + let scripts_dir = swe_bench_run_scripts_dir().map_err(|error| error.message())?; + (Some(evaluator_script), Some(scripts_dir)) } else { - None - }; - let scripts_dir = if evaluation_mode == EVALUATION_MODE_LOCAL_DOCKER { - Some(swe_bench_run_scripts_dir()) - } else { - None + (None, None) }; let repo_path_string = repo_path .map(str::trim) .filter(|value| !value.is_empty()) .map(ToOwned::to_owned); let worktree_path = None; - let command_preview = if evaluation_mode == EVALUATION_MODE_PATCH_ONLY { - swe_bench_patch_only_command_preview(&task, &patch_path) - } else { - swe_bench_command_preview(&resolved_source_path_string, &patch_path, &output_dir) + let command_preview = match (&evaluator_script, &scripts_dir) { + (Some(evaluator_script), Some(scripts_dir)) => swe_bench_command_preview( + evaluator_script, + scripts_dir, + &resolved_source_path_string, + &patch_path, + &output_dir, + ), + _ => swe_bench_patch_only_command_preview(&task, &patch_path), }; Ok(BenchmarkRunPlan { @@ -351,13 +354,15 @@ pub(super) async fn run_swe_bench_process(plan: BenchmarkRunPlan) { } fn swe_bench_command_preview( + evaluator_script: &Path, + scripts_dir: &Path, source_path: &str, patch_path: &Path, output_dir: &Path, ) -> Vec { vec![ benchmark_python_path().display().to_string(), - swe_bench_evaluator_script_path().display().to_string(), + evaluator_script.display().to_string(), "--raw_sample_path".to_string(), source_path.to_string(), "--patch_path".to_string(), @@ -365,7 +370,7 @@ fn swe_bench_command_preview( "--output_dir".to_string(), output_dir.display().to_string(), "--scripts_dir".to_string(), - swe_bench_run_scripts_dir().display().to_string(), + scripts_dir.display().to_string(), "--dockerhub_username".to_string(), SWE_BENCH_PRO_DOCKERHUB_USERNAME.to_string(), "--use_local_docker".to_string(),