diff --git a/.github/workflows/llm-benchmark-periodic.yml b/.github/workflows/llm-benchmark-periodic.yml index e1b0a0e3b28..c8671ac2038 100644 --- a/.github/workflows/llm-benchmark-periodic.yml +++ b/.github/workflows/llm-benchmark-periodic.yml @@ -161,6 +161,11 @@ jobs: ln -sf "$GITHUB_WORKSPACE/target/release/spacetimedb-cli" "$HOME/.local/bin/spacetime" echo "$HOME/.local/bin" >> "$GITHUB_PATH" + - name: Check disk usage before benchmarks + run: | + df -h "$GITHUB_WORKSPACE" "$RUNNER_TEMP" + du -sh "$CARGO_TARGET_DIR" + - name: Run benchmarks env: OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }} @@ -245,6 +250,17 @@ jobs: llm_benchmark run --lang "$LANG" --modes "$MODES" --models "${MODEL_ARGS[@]}" "${EXTRA_ARGS[@]}" fi + - name: Check disk usage after benchmarks + if: always() + run: | + df -h "$GITHUB_WORKSPACE" "$RUNNER_TEMP" + if [ -d "$CARGO_TARGET_DIR" ]; then + du -sh "$CARGO_TARGET_DIR" + fi + if [ -d "$CARGO_TARGET_DIR/llm-runs" ]; then + du -sh "$CARGO_TARGET_DIR/llm-runs" + fi + - name: Upload analysis reports if: always() uses: actions/upload-artifact@v4 diff --git a/tools/xtask-llm-benchmark/src/api/client.rs b/tools/xtask-llm-benchmark/src/api/client.rs index 2c1b0cb6cb3..328693e57d2 100644 --- a/tools/xtask-llm-benchmark/src/api/client.rs +++ b/tools/xtask-llm-benchmark/src/api/client.rs @@ -1,7 +1,7 @@ use anyhow::{anyhow, Context, Result}; use serde_json::json; -use crate::bench::normalize::{canonical_mode, normalize_model_names}; +use crate::bench::normalize::{canonical_mode, ensure_lang, ensure_mode, ensure_model}; use crate::bench::types::{Results, RunOutcome}; use crate::llm::types::Vendor; use crate::llm::ModelRoute; @@ -126,7 +126,7 @@ impl ApiClient { } /// Upload a batch of run outcomes for a single (lang, mode) combination. - /// Normalizes model names and sanitizes volatile fields before upload. + /// Preserves model IDs and sanitizes volatile fields before upload. /// If `analysis` is provided, it is stored in the `llm_benchmark_analysis` table. pub fn upload_batch(&self, mode: &str, outcomes: &[RunOutcome], analysis: Option<&str>) -> Result { if outcomes.is_empty() { @@ -135,24 +135,17 @@ impl ApiClient { let mode = canonical_mode(mode); - // Build in-memory Results so we can normalize model names let mut results = Results::default(); - { - use crate::bench::normalize::{canonical_model_name, ensure_lang, ensure_mode, ensure_model}; - - for r in outcomes { - let lang_v = ensure_lang(&mut results, &r.lang); - let mode_v = ensure_mode(lang_v, mode, Some(r.hash.clone())); - let canonical_name = canonical_model_name(&r.model_name); - let model_v = ensure_model(mode_v, &canonical_name); - model_v.route_api_model = r.route_api_model.clone(); - - let mut sanitized = r.clone(); - sanitized.sanitize_for_commit(); - model_v.tasks.insert(r.task.clone(), sanitized); - } + for r in outcomes { + let lang_v = ensure_lang(&mut results, &r.lang); + let mode_v = ensure_mode(lang_v, mode, Some(r.hash.clone())); + let model_v = ensure_model(mode_v, &r.model_name); + model_v.route_api_model = r.route_api_model.clone(); + + let mut sanitized = r.clone(); + sanitized.sanitize_for_commit(); + model_v.tasks.insert(r.task.clone(), sanitized); } - normalize_model_names(&mut results); let url = format!("{}/api/llm-benchmark-upload", self.base_url); let client = self.client()?; @@ -444,6 +437,78 @@ impl ApiClient { mod tests { use super::*; + #[test] + fn uploads_api_ids_without_converting_them_to_display_names() { + use std::io::{BufRead, BufReader, Read, Write}; + use std::net::TcpListener; + use std::time::{Duration, Instant}; + + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let url = format!("http://{}", listener.local_addr().unwrap()); + listener.set_nonblocking(true).unwrap(); + let server = std::thread::spawn(move || { + let deadline = Instant::now() + Duration::from_secs(10); + let mut stream = loop { + match listener.accept() { + Ok((stream, _)) => break stream, + Err(e) if e.kind() == std::io::ErrorKind::WouldBlock && Instant::now() < deadline => { + std::thread::sleep(Duration::from_millis(10)); + } + Err(e) => panic!("upload request was not received: {e}"), + } + }; + stream.set_read_timeout(Some(Duration::from_secs(10))).unwrap(); + stream.set_write_timeout(Some(Duration::from_secs(10))).unwrap(); + let mut reader = BufReader::new(&stream); + let mut line = String::new(); + reader.read_line(&mut line).unwrap(); + assert_eq!(line, "POST /api/llm-benchmark-upload HTTP/1.1\r\n"); + let mut length = 0; + loop { + line.clear(); + assert_ne!(reader.read_line(&mut line).unwrap(), 0); + if line == "\r\n" { + break; + } + if let Some(value) = line.to_ascii_lowercase().strip_prefix("content-length:") { + length = value.trim().parse().unwrap(); + } + } + let mut body = vec![0; length]; + reader.read_exact(&mut body).unwrap(); + stream.write_all(b"HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: 14\r\nConnection: close\r\n\r\n{\"inserted\":2}").unwrap(); + serde_json::from_slice::(&body).unwrap() + }); + + let outcomes: Vec = ["t_001", "t_002"] + .into_iter() + .map(|task| { + serde_json::from_value(json!({ + "hash": "test", "task": task, "lang": "rust", "golden_published": true, + "model_name": "openai/gpt-5.5", "route_api_model": "gpt-5.5", + "total_tests": 1, "passed_tests": 1, + })) + .unwrap() + }) + .collect(); + assert_eq!( + ApiClient::new(&url, "test") + .unwrap() + .upload_batch("guidelines", &outcomes, Some("analysis")) + .unwrap(), + 2 + ); + let payload = server.join().unwrap(); + let models = payload["models"].as_array().unwrap(); + assert_eq!(models.len(), 1); + assert_eq!(models[0]["name"], "openai/gpt-5.5"); + assert_eq!(models[0]["route_api_model"], "gpt-5.5"); + assert_eq!(models[0]["analysis"], "analysis"); + for task in ["t_001", "t_002"] { + assert_eq!(models[0]["tasks"][task]["model_name"], "openai/gpt-5.5"); + } + } + #[test] fn parses_active_available_model_routes() { let body = json!({ diff --git a/tools/xtask-llm-benchmark/src/bench/normalize.rs b/tools/xtask-llm-benchmark/src/bench/normalize.rs index c49c800f0b0..af534727d9b 100644 --- a/tools/xtask-llm-benchmark/src/bench/normalize.rs +++ b/tools/xtask-llm-benchmark/src/bench/normalize.rs @@ -1,30 +1,5 @@ use crate::bench::types::{LangEntry, ModeEntry, ModelEntry, Results}; -/// Normalize all model names in loaded results and merge duplicates. -pub fn normalize_model_names(root: &mut Results) { - for lang in &mut root.languages { - for mode in &mut lang.modes { - let mut merged: Vec = Vec::new(); - for mut model in mode.models.drain(..) { - let canonical = canonical_model_name(&model.name); - model.name = canonical; - if let Some(existing) = merged.iter_mut().find(|m| m.name == model.name) { - // Merge tasks from duplicate into existing entry - for (task_id, outcome) in model.tasks { - existing.tasks.insert(task_id, outcome); - } - if existing.route_api_model.is_none() { - existing.route_api_model = model.route_api_model; - } - } else { - merged.push(model); - } - } - mode.models = merged; - } - } -} - pub fn ensure_lang<'a>(root: &'a mut Results, lang: &str) -> &'a mut LangEntry { if let Some(i) = root.languages.iter().position(|x| x.lang == lang) { return &mut root.languages[i]; @@ -70,27 +45,3 @@ pub fn canonical_mode(mode: &str) -> &str { other => other, } } - -/// Normalize model names so that OpenRouter-style IDs and case variants -/// resolve to the canonical display name from model_routes. -pub fn canonical_model_name(name: &str) -> String { - use crate::llm::model_routes::default_model_routes; - let lower = name.to_ascii_lowercase(); - for route in default_model_routes() { - // Match by openrouter model id (e.g. "anthropic/claude-sonnet-4.6") - if let Some(ref or) = route.openrouter_model - && lower == or.to_ascii_lowercase() - { - return route.display_name.to_string(); - } - // Match by api model id (e.g. "claude-sonnet-4-6") - if lower == route.api_model.to_ascii_lowercase() { - return route.display_name.to_string(); - } - // Match by case-insensitive display name (e.g. "claude sonnet 4.6") - if lower == route.display_name.to_ascii_lowercase() { - return route.display_name.to_string(); - } - } - name.to_string() -} diff --git a/tools/xtask-llm-benchmark/src/bench/runner.rs b/tools/xtask-llm-benchmark/src/bench/runner.rs index f993f80c883..e430a132501 100644 --- a/tools/xtask-llm-benchmark/src/bench/runner.rs +++ b/tools/xtask-llm-benchmark/src/bench/runner.rs @@ -69,32 +69,37 @@ pub async fn ensure_goldens_built_once( build_goldens_only_for_lang(host, bench_root, lang, selectors, golden_scope).await } -async fn publish_rust_async( - publisher: SpacetimeRustPublisher, - host_url: String, - wdir: PathBuf, - db: String, - clear_database: bool, -) -> Result { - task::spawn_blocking(move || publisher.publish(&host_url, &wdir, &db, clear_database)).await? -} -async fn publish_cs_async( - publisher: DotnetPublisher, - host_url: String, - wdir: PathBuf, - db: String, - clear_database: bool, -) -> Result { - task::spawn_blocking(move || publisher.publish(&host_url, &wdir, &db, clear_database)).await? +fn with_build_cleanup(project: &Path, lang: Lang, publish: impl FnOnce() -> Result) -> Result { + let result = publish(); + // Only task-local outputs: never follow the shared Cargo target override or shared caches. + let directories: &[&str] = match lang { + Lang::Rust => &["target"], + Lang::CSharp => &["bin", "obj", ".nuget"], + Lang::TypeScript => &["node_modules", "dist"], + }; + for directory in directories { + let path = project.join(directory); + if let Err(error) = fs::remove_dir_all(&path) + && error.kind() != std::io::ErrorKind::NotFound + { + eprintln!("[cleanup] failed to remove build output {}: {error}", path.display()); + } + } + result } -async fn publish_ts_async( - publisher: TypeScriptPublisher, + +async fn publish_async( + publisher: impl Publisher + 'static, + lang: Lang, host_url: String, wdir: PathBuf, db: String, clear_database: bool, ) -> Result { - task::spawn_blocking(move || publisher.publish(&host_url, &wdir, &db, clear_database)).await? + task::spawn_blocking(move || { + with_build_cleanup(&wdir, lang, || publisher.publish(&host_url, &wdir, &db, clear_database)) + }) + .await? } async fn delete_database_async(database: PublishedDatabase) -> Result<()> { @@ -177,8 +182,9 @@ impl TaskRunner { let host_url = params.host.unwrap_or_else(|| "local".to_owned()); let database = match params.lang { Lang::Rust => { - publish_rust_async( + publish_async( self.rust_publisher, + params.lang, host_url, wdir, params.db_name, @@ -187,10 +193,26 @@ impl TaskRunner { .await? } Lang::CSharp => { - publish_cs_async(self.cs_publisher, host_url, wdir, params.db_name, params.clear_database).await? + publish_async( + self.cs_publisher, + params.lang, + host_url, + wdir, + params.db_name, + params.clear_database, + ) + .await? } Lang::TypeScript => { - publish_ts_async(self.ts_publisher, host_url, wdir, params.db_name, params.clear_database).await? + publish_async( + self.ts_publisher, + params.lang, + host_url, + wdir, + params.db_name, + params.clear_database, + ) + .await? } }; @@ -428,7 +450,7 @@ impl TaskRunner { hash: cfg.hash.to_string(), task: task_id.clone(), lang: cfg.lang_name.to_string(), - model_name: cfg.route.display_name.to_string(), + model_name: cfg.route.model_id(), vendor: cfg.route.vendor.slug().to_string(), golden_published: publish_error.is_none(), total_tests: total_tasks as u32, @@ -1261,7 +1283,7 @@ fn build_fail_outcome( golden_published: false, category: Some(category), - model_name: route.display_name.to_string(), + model_name: route.model_id(), total_tests: 1, passed_tests: 0, @@ -1354,3 +1376,46 @@ fn normalize_task_selector(raw: &str) -> Result { } bail!("invalid task selector: {raw}") } + +#[cfg(test)] +mod build_cleanup_tests { + use super::*; + + #[test] + fn preserves_sources_and_shared_files_after_success_and_failure() { + let root = std::env::temp_dir().join(format!("llm-build-cleanup-{}", std::process::id())); + for (lang, directories) in [ + (Lang::Rust, vec!["target"]), + (Lang::CSharp, vec!["bin", "obj", ".nuget"]), + (Lang::TypeScript, vec!["node_modules", "dist"]), + ] { + for succeeds in [true, false] { + let project = root.join("project"); + fs::create_dir_all(&project).unwrap(); + fs::write(project.join("source"), "retain source").unwrap(); + fs::write(root.join("shared-cache"), "retain cache").unwrap(); + for directory in &directories { + fs::create_dir_all(project.join(directory)).unwrap(); + fs::write(project.join(directory).join("artifact"), "build output").unwrap(); + } + let result = with_build_cleanup(&project, lang, || { + assert!(directories.iter().all(|dir| project.join(dir).exists())); + if succeeds { + Ok(()) + } else { + Err(anyhow!("build failed")) + } + }); + assert_eq!(result.is_ok(), succeeds); + if let Err(error) = result { + assert_eq!(error.to_string(), "build failed"); + } + assert!(directories.iter().all(|dir| !project.join(dir).exists())); + assert_eq!(fs::read_to_string(project.join("source")).unwrap(), "retain source"); + assert_eq!(fs::read_to_string(root.join("shared-cache")).unwrap(), "retain cache"); + with_build_cleanup(&project, lang, || Ok(())).unwrap(); + } + } + fs::remove_dir_all(root).unwrap(); + } +} diff --git a/tools/xtask-llm-benchmark/src/llm/model_routes.rs b/tools/xtask-llm-benchmark/src/llm/model_routes.rs index 7f7ae93b66c..e26310ac9bf 100644 --- a/tools/xtask-llm-benchmark/src/llm/model_routes.rs +++ b/tools/xtask-llm-benchmark/src/llm/model_routes.rs @@ -63,6 +63,22 @@ static DEFAULT_ROUTES: LazyLock> = LazyLock::new(|| { }); impl ModelRoute { + /// Stable result key shared by direct and OpenRouter routes, independent of display labels. + pub fn model_id(&self) -> String { + if let Some(id) = &self.openrouter_model { + return id.clone(); + } + if self.api_model.contains('/') { + return self.api_model.clone(); + } + let prefix = match self.vendor { + Vendor::Xai => "x-ai", + Vendor::Meta | Vendor::OpenRouter => return self.api_model.clone(), + vendor => vendor.slug(), + }; + format!("{prefix}/{}", self.api_model) + } + pub fn new(display_name: &str, vendor: Vendor, api_model: &str, openrouter_model: Option<&str>) -> Self { Self { display_name: display_name.to_string(), @@ -76,3 +92,35 @@ impl ModelRoute { pub fn default_model_routes() -> &'static [ModelRoute] { &DEFAULT_ROUTES } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn model_identity_is_independent_of_labels_and_route_source() { + let mut routes = vec![ + ModelRoute::new("GPT-5.5", Vendor::OpenAi, "gpt-5.5", Some("openai/gpt-5.5")), + ModelRoute::new("openai/gpt-5.5", Vendor::OpenRouter, "openai/gpt-5.5", None), + ModelRoute::new("OpenAI: GPT-5.5", Vendor::OpenRouter, "openai/gpt-5.5", None), + ModelRoute::new("Direct model", Vendor::OpenAi, "gpt-5.5", None), + ]; + for route in &mut routes { + assert_eq!(route.model_id(), "openai/gpt-5.5"); + route.display_name = "Changed label".into(); + assert_eq!(route.model_id(), "openai/gpt-5.5"); + } + assert_ne!( + ModelRoute::new("Same label", Vendor::OpenAi, "test", None).model_id(), + ModelRoute::new("Same label", Vendor::Anthropic, "test", None).model_id(), + ); + assert_eq!( + ModelRoute::new("Grok", Vendor::Xai, "grok-test", None).model_id(), + "x-ai/grok-test" + ); + assert_eq!( + ModelRoute::new("Llama", Vendor::Meta, "meta-llama/test", None).model_id(), + "meta-llama/test" + ); + } +} diff --git a/tools/xtask-llm-benchmark/src/llm/provider.rs b/tools/xtask-llm-benchmark/src/llm/provider.rs index 1dba17764b9..fbec5b7e674 100644 --- a/tools/xtask-llm-benchmark/src/llm/provider.rs +++ b/tools/xtask-llm-benchmark/src/llm/provider.rs @@ -118,10 +118,7 @@ impl LlmProvider for RouterProvider { impl RouterProvider { fn resolve_client<'a>(&'a self, route: &ModelRoute, search_enabled: bool) -> Result> { if search_enabled { - let base_model = route - .openrouter_model - .clone() - .unwrap_or_else(|| openrouter_model_id(route.vendor, &route.api_model)); + let base_model = route.model_id(); return self.resolve_openrouter(format!("{base_model}:online"), None, true); } @@ -152,10 +149,7 @@ impl RouterProvider { }); } - let model = route - .openrouter_model - .clone() - .unwrap_or_else(|| openrouter_model_id(route.vendor, &route.api_model)); + let model = route.model_id(); self.resolve_openrouter(model, Some(vendor.display_name()), false) } @@ -180,21 +174,3 @@ impl RouterProvider { }) } } - -/// Map a vendor + bare model id to the `vendor/model` namespace that OpenRouter requires. -/// If the model id already contains `/` it is returned as-is (e.g. `google/gemini-3.1-pro-preview`). -fn openrouter_model_id(vendor: Vendor, api_model: &str) -> String { - if api_model.contains('/') { - return api_model.to_string(); - } - let prefix = match vendor { - Vendor::Anthropic => "anthropic", - Vendor::OpenAi => "openai", - Vendor::Xai => "x-ai", - Vendor::DeepSeek => "deepseek", - Vendor::Google => "google", - // Meta rows already carry a full `vendor/model` id (caught by the `/` check above). - Vendor::Meta | Vendor::OpenRouter => return api_model.to_string(), - }; - format!("{}/{}", prefix, api_model) -}