From 7472d14503b6ba51083ab4456901a1175709a0c9 Mon Sep 17 00:00:00 2001 From: Perry Hertler Date: Thu, 20 Aug 2026 11:59:11 -0500 Subject: [PATCH 1/2] perf: skip the project build for validate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit validate resolves ownership from the config and the CODEOWNERS file only — it never touches Runner's Project. But it went through Runner::new, which walks every tracked file in the repo first. On a 130k-file monorepo that is a ~2.1s fixed cost paid for nothing, on the code path documented as "fast mode for git hooks". Extracts validate_files into a free function and routes api::validate to it directly when paths are given, mirroring the bypass for_file already uses for single-file queries. Tradeoff, deliberately not hidden: the fast path no longer creates or persists the cache, so it does not warm it for a following command, and it cannot surface project-build IO errors. --no-cache is a no-op there. Co-Authored-By: Claude Fable 5 --- src/runner.rs | 111 +++++++++++++++++++++++++--------------------- src/runner/api.rs | 16 +++++++ 2 files changed, 76 insertions(+), 51 deletions(-) diff --git a/src/runner.rs b/src/runner.rs index 5562979..7d4a405 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -79,6 +79,65 @@ pub(crate) fn resolve_codeowners_file_path(run_config: &RunConfig, config: &Conf run_config.project_root.join(&config.codeowners_path).join("CODEOWNERS") } +/// Validates ownership for an explicit list of paths. +/// +/// Resolves ownership entirely from the config and the CODEOWNERS file, so it +/// needs no `Project` and therefore no project build. Lives outside `Runner` so +/// the `validate ` entry point can call it without constructing one. +pub(crate) fn validate_file_paths(run_config: &RunConfig, config: &Config, file_paths: Vec) -> RunResult { + let mut unowned_files = Vec::new(); + let mut io_errors = Vec::new(); + + // Filter files based on owned_globs and unowned_globs configuration + // Only validate files that match owned_globs and don't match unowned_globs + let filtered_paths: Vec = file_paths + .into_iter() + .filter(|file_path| { + // Convert to relative path for glob matching + let path = Path::new(file_path); + let relative_path = if path.is_absolute() { + path.strip_prefix(&run_config.project_root).unwrap_or(path) + } else { + path + }; + + // Mirror the filtering applied by ProjectBuilder when walking the project + matches_globs(relative_path, &config.owned_globs) && !matches_globs(relative_path, &config.unowned_globs) + }) + .collect(); + + debug_span!("per_file_query").in_scope(|| { + for file_path in filtered_paths { + match team_for_file_from_codeowners(run_config, &file_path) { + Ok(Some(_)) => {} + Ok(None) => unowned_files.push(file_path), + Err(err) => io_errors.push(format!("{}: {}", file_path, err)), + } + } + }); + + if !unowned_files.is_empty() { + let validation_errors = std::iter::once("Unowned files detected:".to_string()) + .chain(unowned_files.into_iter().map(|file| format!(" {}", file))) + .collect(); + + return RunResult { + validation_errors, + io_errors, + ..Default::default() + }; + } + + if !io_errors.is_empty() { + return RunResult { + io_errors, + ..Default::default() + }; + } + + RunResult::default() +} + impl Runner { pub fn new(run_config: &RunConfig) -> Result> { let config = debug_span!("config_load").in_scope(|| config_from_run_config(run_config))?; @@ -143,57 +202,7 @@ impl Runner { } fn validate_files(&self, file_paths: Vec) -> RunResult { - let mut unowned_files = Vec::new(); - let mut io_errors = Vec::new(); - - // Filter files based on owned_globs and unowned_globs configuration - // Only validate files that match owned_globs and don't match unowned_globs - let filtered_paths: Vec = file_paths - .into_iter() - .filter(|file_path| { - // Convert to relative path for glob matching - let path = Path::new(file_path); - let relative_path = if path.is_absolute() { - path.strip_prefix(&self.run_config.project_root).unwrap_or(path) - } else { - path - }; - - // Mirror the filtering applied by ProjectBuilder when walking the project - matches_globs(relative_path, &self.config.owned_globs) && !matches_globs(relative_path, &self.config.unowned_globs) - }) - .collect(); - - debug_span!("per_file_query").in_scope(|| { - for file_path in filtered_paths { - match team_for_file_from_codeowners(&self.run_config, &file_path) { - Ok(Some(_)) => {} - Ok(None) => unowned_files.push(file_path), - Err(err) => io_errors.push(format!("{}: {}", file_path, err)), - } - } - }); - - if !unowned_files.is_empty() { - let validation_errors = std::iter::once("Unowned files detected:".to_string()) - .chain(unowned_files.into_iter().map(|file| format!(" {}", file))) - .collect(); - - return RunResult { - validation_errors, - io_errors, - ..Default::default() - }; - } - - if !io_errors.is_empty() { - return RunResult { - io_errors, - ..Default::default() - }; - } - - RunResult::default() + validate_file_paths(&self.run_config, &self.config, file_paths) } pub fn generate(&self, git_stage: bool) -> RunResult { diff --git a/src/runner/api.rs b/src/runner/api.rs index 0c09b4a..3b9d5c0 100644 --- a/src/runner/api.rs +++ b/src/runner/api.rs @@ -18,6 +18,22 @@ pub fn for_team(run_config: &RunConfig, team_name: &str) -> RunResult { } pub fn validate(run_config: &RunConfig, file_paths: Vec) -> RunResult { + // Fast path for `validate `. Ownership for an explicit path list is + // resolved from the config and the CODEOWNERS file alone, so building the + // whole project first — a full walk of every tracked file — is dead work. + // + // Tradeoff: this path no longer creates or persists the cache, so it does not + // warm it for a subsequent command, and it cannot surface project-build IO + // errors. `--no-cache` has no effect here. + if !file_paths.is_empty() { + return match config_from_run_config(run_config) { + Ok(config) => super::validate_file_paths(run_config, &config, file_paths), + Err(err) => RunResult { + io_errors: vec![format!("{:?}", err)], + ..Default::default() + }, + }; + } run(run_config, |runner| runner.validate(file_paths)) } From fa68a94c20bb6788545c3c8f70ca3391d5dd2b15 Mon Sep 17 00:00:00 2001 From: Perry Hertler Date: Thu, 20 Aug 2026 12:01:20 -0500 Subject: [PATCH 2/2] perf: batch the CODEOWNERS query on the no-project-build fast path Combines O1 (one batched CODEOWNERS query instead of one per path) with O2 (skip the project build for validate ). The two wins are independent: O2 removes the ~2.1s fixed project build, O1 removes the ~9.5ms/file linear term. Together they take validate on 1000 files from 13.1s to 0.17s. Co-Authored-By: Claude Fable 5 --- src/runner.rs | 71 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 48 insertions(+), 23 deletions(-) diff --git a/src/runner.rs b/src/runner.rs index 7d4a405..312f73c 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -85,16 +85,17 @@ pub(crate) fn resolve_codeowners_file_path(run_config: &RunConfig, config: &Conf /// needs no `Project` and therefore no project build. Lives outside `Runner` so /// the `validate ` entry point can call it without constructing one. pub(crate) fn validate_file_paths(run_config: &RunConfig, config: &Config, file_paths: Vec) -> RunResult { - let mut unowned_files = Vec::new(); - let mut io_errors = Vec::new(); - // Filter files based on owned_globs and unowned_globs configuration // Only validate files that match owned_globs and don't match unowned_globs - let filtered_paths: Vec = file_paths + // + // Each surviving path is kept alongside its project-relative form: the + // relative form is what the CODEOWNERS query is keyed by, while the original + // is what gets reported back to the caller. + let filtered: Vec<(String, String)> = file_paths .into_iter() - .filter(|file_path| { + .filter_map(|file_path| { // Convert to relative path for glob matching - let path = Path::new(file_path); + let path = Path::new(&file_path); let relative_path = if path.is_absolute() { path.strip_prefix(&run_config.project_root).unwrap_or(path) } else { @@ -102,19 +103,51 @@ pub(crate) fn validate_file_paths(run_config: &RunConfig, config: &Config, file_ }; // Mirror the filtering applied by ProjectBuilder when walking the project - matches_globs(relative_path, &config.owned_globs) && !matches_globs(relative_path, &config.unowned_globs) + if matches_globs(relative_path, &config.owned_globs) && !matches_globs(relative_path, &config.unowned_globs) { + let relative = relative_path.to_string_lossy().into_owned(); + Some((file_path, relative)) + } else { + None + } }) .collect(); - debug_span!("per_file_query").in_scope(|| { - for file_path in filtered_paths { - match team_for_file_from_codeowners(run_config, &file_path) { - Ok(Some(_)) => {} - Ok(None) => unowned_files.push(file_path), - Err(err) => io_errors.push(format!("{}: {}", file_path, err)), - } + if filtered.is_empty() { + return RunResult::default(); + } + + // One batched query for every path, rather than one query per path. The + // per-path version re-read and re-parsed the entire CODEOWNERS file each time + // (`parse_codeowners_entries` is not memoized), costing ~9.5ms per file + // against an 18k-line CODEOWNERS. The batch function already parallelizes + // across the paths it is given. + let codeowners_file_path = resolve_codeowners_file_path(run_config, config); + let relative_paths: Vec = filtered.iter().map(|(_, relative)| relative.clone()).collect(); + let teams = match debug_span!("per_file_query").in_scope(|| { + crate::ownership::codeowners_query::teams_for_files_from_codeowners( + &run_config.project_root, + &codeowners_file_path, + &config.team_file_glob, + &relative_paths, + ) + }) { + Ok(teams) => teams, + // The per-path loop could attribute an IO failure to one specific file. A + // batched read either succeeds or fails for the whole set, so the error is + // no longer per-path. + Err(err) => { + return RunResult { + io_errors: vec![err], + ..Default::default() + }; } - }); + }; + + let unowned_files: Vec = filtered + .into_iter() + .filter(|(_, relative)| teams.get(relative).is_none_or(Option::is_none)) + .map(|(original, _)| original) + .collect(); if !unowned_files.is_empty() { let validation_errors = std::iter::once("Unowned files detected:".to_string()) @@ -123,14 +156,6 @@ pub(crate) fn validate_file_paths(run_config: &RunConfig, config: &Config, file_ return RunResult { validation_errors, - io_errors, - ..Default::default() - }; - } - - if !io_errors.is_empty() { - return RunResult { - io_errors, ..Default::default() }; }