From 7bec8f0efa0306ff68156aaaa2fd4cb29ad12a14 Mon Sep 17 00:00:00 2001 From: ajianaz Date: Thu, 2 Jul 2026 12:06:37 +0700 Subject: [PATCH] perf: resolve scan/review pipeline performance bottlenecks (P2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix 10 performance issues — all 'repeated work in loops' patterns: #41 - Pre-compile regex patterns once per rule instead of per line #5 - Batch DB queries in handle_find_affected_tests (single IN clause) #13 - Batch symbol fetch for Affected command (single query) #14 - Prepare SQL statement once outside test-pattern loop #59 - Early cutoff in secrets scanner when max_findings reached #70 - Break out of all loops when symbol cap hit per file #67 - Cache file reads during sibling search in resolver #3 - Reuse single Tokio runtime for MCP tool calls (static LazyLock) #22 - Static LazyLock regexes in detect_function_entry #24 - Single transaction for prune_deleted instead of per-file Refs #335 --- src/engine/context/extraction.rs | 18 ++++-- src/engine/context/resolver.rs | 80 ++++++++++++++++++----- src/engine/rules/builtin.rs | 12 ++++ src/engine/rules/matching.rs | 15 +++-- src/engine/rules/mod.rs | 5 ++ src/engine/rules/types.rs | 22 ++++++- src/engine/secrets_scanner.rs | 12 ++++ src/engine/types.rs | 3 +- src/index/extract.rs | 25 ++++---- src/index/mod.rs | 17 +++-- src/main.rs | 47 ++++++++------ src/mcp/tools.rs | 105 +++++++++++++++++++------------ 12 files changed, 257 insertions(+), 104 deletions(-) diff --git a/src/engine/context/extraction.rs b/src/engine/context/extraction.rs index ad871c5..b24e874 100644 --- a/src/engine/context/extraction.rs +++ b/src/engine/context/extraction.rs @@ -274,14 +274,15 @@ pub fn extract_symbols_from_diff( continue; } + // Early cutoff: stop processing lines for this file once cap is reached + if file_symbol_count >= MAX_SYMBOLS_PER_FILE { + break; + } + let language = &file.language; let symbols = extract_symbols_from_line(&line.content, language); for sym in symbols { - if file_symbol_count >= MAX_SYMBOLS_PER_FILE { - break; - } - let key = format!("{sym}"); if seen.insert((key, file_path.to_string())) { all_symbols.push(ExtractedSymbol { @@ -291,8 +292,17 @@ pub fn extract_symbols_from_diff( raw: line.content.clone(), }); file_symbol_count += 1; + if file_symbol_count >= MAX_SYMBOLS_PER_FILE { + break; + } } } + if file_symbol_count >= MAX_SYMBOLS_PER_FILE { + break; + } + } + if file_symbol_count >= MAX_SYMBOLS_PER_FILE { + break; } } diff --git a/src/engine/context/resolver.rs b/src/engine/context/resolver.rs index 79df8bd..1ae197a 100644 --- a/src/engine/context/resolver.rs +++ b/src/engine/context/resolver.rs @@ -145,6 +145,9 @@ fn resolve_symbols( let mut entries = Vec::new(); let mut seen_files: HashMap> = HashMap::new(); // track line ranges per file + // File content cache to avoid re-reading the same files from disk + let mut file_cache: HashMap = HashMap::new(); + for sym in symbols { // Determine the language from the file extension let lang = Path::new(&sym.file) @@ -154,8 +157,12 @@ fn resolve_symbols( let resolved = match &sym.kind { SymbolKind::Import(path) => resolve_import(path, &sym.file, lang, project_root), - SymbolKind::FunctionCall(name) => resolve_function(name, &sym.file, lang, project_root), - SymbolKind::TypeRef(name) => resolve_type(name, &sym.file, lang, project_root), + SymbolKind::FunctionCall(name) => { + resolve_function(name, &sym.file, lang, project_root, &mut file_cache) + } + SymbolKind::TypeRef(name) => { + resolve_type(name, &sym.file, lang, project_root, &mut file_cache) + } }; for entry in resolved { @@ -344,6 +351,7 @@ fn resolve_function( source_file: &str, lang: &str, project_root: &Path, + file_cache: &mut HashMap, ) -> Vec { let mut entries = Vec::new(); @@ -370,7 +378,9 @@ fn resolve_function( continue; } - if let Some((start, end)) = find_fn_in_file(&path, name, "rs") { + if let Some((start, end)) = + find_fn_in_file_cached(&path, name, "rs", file_cache) + { entries.push(ContextEntry { file: rel_str, line_start: start, @@ -397,7 +407,9 @@ fn resolve_function( continue; } - if let Some((start, end)) = find_fn_in_file(&path, name, "py") { + if let Some((start, end)) = + find_fn_in_file_cached(&path, name, "py", file_cache) + { entries.push(ContextEntry { file: rel_str, line_start: start, @@ -421,7 +433,8 @@ fn resolve_function( continue; } - if let Some((start, end)) = find_fn_generic(&path, name) { + if let Some((start, end)) = find_fn_generic_cached(&path, name, file_cache) + { entries.push(ContextEntry { file: rel_str, line_start: start, @@ -445,6 +458,7 @@ fn resolve_type( source_file: &str, lang: &str, project_root: &Path, + file_cache: &mut HashMap, ) -> Vec { let search_dir = Path::new(source_file).parent().unwrap_or(Path::new("")); let mut entries = Vec::new(); @@ -466,7 +480,8 @@ fn resolve_type( continue; } - if let Some((start, end)) = find_pattern_in_file(&path, &pattern) { + if let Some((start, end)) = find_pattern_in_file_cached(&path, &pattern, file_cache) + { entries.push(ContextEntry { file: rel_str, line_start: start, @@ -482,37 +497,68 @@ fn resolve_type( entries } -/// Find a function definition in a file and return (start_line, end_line). -fn find_fn_in_file(path: &Path, name: &str, lang: &str) -> Option<(u32, u32)> { - let content = std::fs::read_to_string(path).ok()?; +/// Cached version of `find_fn_in_file` — avoids re-reading files from disk. +fn find_fn_in_file_cached( + path: &Path, + name: &str, + lang: &str, + file_cache: &mut HashMap, +) -> Option<(u32, u32)> { + let content = get_file_cached(path, file_cache)?; let pattern = match lang { "rs" => format!("fn {name}"), "py" => format!("def {name}"), - _ => return find_fn_generic(path, name), - }; - - let max_lines = match lang { - "rs" => MAX_FN_LINES, - "py" => MAX_FN_LINES, - _ => MAX_FN_LINES, + _ => return find_fn_generic_cached(path, name, file_cache), }; - find_pattern_with_body(&content, &pattern, max_lines) + find_pattern_with_body(&content, &pattern, MAX_FN_LINES) } /// Generic function search (for languages without specific patterns). +#[allow(dead_code)] fn find_fn_generic(path: &Path, name: &str) -> Option<(u32, u32)> { let content = std::fs::read_to_string(path).ok()?; find_pattern_with_body(&content, &format!("fn {name}"), MAX_FN_LINES) } +/// Cached version of `find_fn_generic`. +fn find_fn_generic_cached( + path: &Path, + name: &str, + file_cache: &mut HashMap, +) -> Option<(u32, u32)> { + let content = get_file_cached(path, file_cache)?; + find_pattern_with_body(&content, &format!("fn {name}"), MAX_FN_LINES) +} + /// Find a pattern (like `struct Foo`) and determine its extent. +#[allow(dead_code)] fn find_pattern_in_file(path: &Path, pattern: &str) -> Option<(u32, u32)> { let content = std::fs::read_to_string(path).ok()?; find_pattern_with_body(&content, pattern, MAX_TYPE_LINES) } +/// Cached version of `find_pattern_in_file`. +fn find_pattern_in_file_cached( + path: &Path, + pattern: &str, + file_cache: &mut HashMap, +) -> Option<(u32, u32)> { + let content = get_file_cached(path, file_cache)?; + find_pattern_with_body(&content, pattern, MAX_TYPE_LINES) +} + +/// Read a file from disk, using the cache if available. +fn get_file_cached(path: &Path, cache: &mut HashMap) -> Option { + if let Some(content) = cache.get(path) { + return Some(content.clone()); + } + let content = std::fs::read_to_string(path).ok()?; + cache.insert(path.to_path_buf(), content.clone()); + Some(content) +} + /// Find a pattern in content and estimate the block extent by counting braces/indents. fn find_pattern_with_body(content: &str, pattern: &str, max_lines: usize) -> Option<(u32, u32)> { let mut start_line: Option = None; diff --git a/src/engine/rules/builtin.rs b/src/engine/rules/builtin.rs index 1b682bc..96011cb 100644 --- a/src/engine/rules/builtin.rs +++ b/src/engine/rules/builtin.rs @@ -14,6 +14,7 @@ pub fn builtin_rules() -> Vec { message: "Possible hardcoded secret/credential detected. Use environment variables or a secrets manager.".to_string(), languages: vec!["all".to_string()], exclude: vec!["rules/".to_string(), "tests/".to_string(), "test/".to_string()], + ..Default::default() }, CustomRule { id: "sec-sql-concat".to_string(), @@ -23,6 +24,7 @@ pub fn builtin_rules() -> Vec { message: "Possible SQL injection via string concatenation in query. Use parameterized queries.".to_string(), languages: vec!["all".to_string()], exclude: vec!["rules/".to_string(), "tests/".to_string(), "test/".to_string()], + ..Default::default() }, CustomRule { id: "sec-hardcoded-url".to_string(), @@ -32,6 +34,7 @@ pub fn builtin_rules() -> Vec { message: "Insecure HTTP URL detected (not https). Use HTTPS for all external connections.".to_string(), languages: vec!["all".to_string()], exclude: vec!["rules/".to_string(), "tests/".to_string(), "test/".to_string()], + ..Default::default() }, CustomRule { id: "sec-tls-disabled".to_string(), @@ -40,6 +43,7 @@ pub fn builtin_rules() -> Vec { message: "TLS verification disabled. This allows man-in-the-middle attacks.".to_string(), languages: vec!["rs".to_string(), "py".to_string(), "go".to_string()], exclude: vec!["rules/".to_string(), "tests/".to_string(), "test/".to_string()], + ..Default::default() }, // --- Bugs --- CustomRule { @@ -49,6 +53,7 @@ pub fn builtin_rules() -> Vec { message: "Use of `.unwrap()` can panic in production. Handle the error properly.".to_string(), languages: vec!["rs".to_string()], exclude: vec!["tests/".to_string(), "test/".to_string()], + ..Default::default() }, CustomRule { id: "bug-expect".to_string(), @@ -58,6 +63,7 @@ pub fn builtin_rules() -> Vec { message: "Use of `.expect()` can panic in production. Consider proper error handling.".to_string(), languages: vec!["rs".to_string()], exclude: vec!["tests/".to_string(), "test/".to_string()], + ..Default::default() }, CustomRule { id: "bug-println".to_string(), @@ -66,6 +72,7 @@ pub fn builtin_rules() -> Vec { message: "Debug output macro found. Remove `println!`/`dbg!`/`print!` before merging.".to_string(), languages: vec!["rs".to_string()], exclude: vec!["tests/".to_string(), "test/".to_string()], + ..Default::default() }, CustomRule { id: "bug-todo".to_string(), @@ -74,6 +81,7 @@ pub fn builtin_rules() -> Vec { message: "TODO/FIXME/HACK/XXX comment found. Consider resolving before merge.".to_string(), languages: vec!["all".to_string()], exclude: vec![], + ..Default::default() }, CustomRule { id: "bug-console-log".to_string(), @@ -82,6 +90,7 @@ pub fn builtin_rules() -> Vec { message: "Console logging statement found. Remove before merging to production.".to_string(), languages: vec!["js".to_string(), "ts".to_string()], exclude: vec!["tests/".to_string(), "test/".to_string()], + ..Default::default() }, CustomRule { id: "bug-hardcoded-port".to_string(), @@ -90,6 +99,7 @@ pub fn builtin_rules() -> Vec { message: "Hardcoded port number detected. Consider using environment variables or config.".to_string(), languages: vec!["all".to_string()], exclude: vec![], + ..Default::default() }, // --- Quality --- CustomRule { @@ -99,6 +109,7 @@ pub fn builtin_rules() -> Vec { message: "Error result discarded with `let _ =`. Consider handling the error explicitly.".to_string(), languages: vec!["all".to_string()], exclude: vec![], + ..Default::default() }, CustomRule { id: "qual-clone".to_string(), @@ -107,6 +118,7 @@ pub fn builtin_rules() -> Vec { message: "Use of `.clone()` detected. Consider borrowing or ownership transfer.".to_string(), languages: vec!["rs".to_string()], exclude: vec![], + ..Default::default() }, ] } diff --git a/src/engine/rules/matching.rs b/src/engine/rules/matching.rs index 7b5dd25..444b9ba 100644 --- a/src/engine/rules/matching.rs +++ b/src/engine/rules/matching.rs @@ -33,10 +33,10 @@ pub fn matches_exclude(rule: &CustomRule, file_path: &str) -> bool { /// Check if a rule's regex pattern matches a line of code. pub fn match_rule_against_line(rule: &CustomRule, line: &str) -> bool { - match regex::Regex::new(&rule.pattern) { - Ok(re) => re.is_match(line), - Err(e) => { - debug!(rule_id = %rule.id, error = %e, "invalid regex in rule, skipping"); + match &rule.compiled_pattern { + Some(re) => re.is_match(line), + None => { + debug!(rule_id = %rule.id, "regex not compiled, skipping"); false } } @@ -112,14 +112,17 @@ mod tests { use crate::engine::Severity; fn make_rule(pattern: &str, languages: &[&str], exclude: &[&str]) -> CustomRule { - CustomRule { + let mut rule = CustomRule { id: "test-rule".to_string(), pattern: pattern.to_string(), severity: Severity::Minor, message: "test".to_string(), languages: languages.iter().map(|s| s.to_string()).collect(), exclude: exclude.iter().map(|s| s.to_string()).collect(), - } + compiled_pattern: None, + }; + rule.ensure_compiled(); + rule } #[test] diff --git a/src/engine/rules/mod.rs b/src/engine/rules/mod.rs index c8028f8..f4f7bd6 100644 --- a/src/engine/rules/mod.rs +++ b/src/engine/rules/mod.rs @@ -32,6 +32,11 @@ pub fn run_rules(chunks: &[FileChunk], config: &RulesConfig) -> Vec let mut all_rules = builtin::builtin_rules(); all_rules.extend(config.custom_rules.clone()); + // Compile regex patterns once before the matching loop (Fix #41) + for rule in &mut all_rules { + rule.ensure_compiled(); + } + debug!( rule_count = all_rules.len(), "running rules against diff chunks" diff --git a/src/engine/rules/types.rs b/src/engine/rules/types.rs index dae6d8d..59d74ba 100644 --- a/src/engine/rules/types.rs +++ b/src/engine/rules/types.rs @@ -25,7 +25,7 @@ impl Default for RulesConfig { } /// A user-defined or built-in rule definition. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct CustomRule { /// Unique rule identifier (e.g., `"sec-hardcoded-secret"`). pub id: String, @@ -39,6 +39,26 @@ pub struct CustomRule { pub languages: Vec, /// Glob patterns for file paths to exclude from this rule. pub exclude: Vec, + /// Pre-compiled regex for the pattern (populated after rule assembly). + #[serde(skip, default)] + pub compiled_pattern: Option, +} + +impl CustomRule { + /// Compile the rule's pattern into a cached regex, if not already compiled. + /// Returns `true` if compilation succeeded, `false` if the pattern is invalid. + pub fn ensure_compiled(&mut self) -> bool { + if self.compiled_pattern.is_some() { + return true; + } + match regex::Regex::new(&self.pattern) { + Ok(re) => { + self.compiled_pattern = Some(re); + true + } + Err(_) => false, + } + } } /// A single finding produced by a rule. diff --git a/src/engine/secrets_scanner.rs b/src/engine/secrets_scanner.rs index 9d9a881..398ab55 100644 --- a/src/engine/secrets_scanner.rs +++ b/src/engine/secrets_scanner.rs @@ -149,10 +149,22 @@ pub fn scan_secrets(chunks: &[FileChunk], max_findings: usize) -> Vec= max_findings { + break; + } } } + if findings.len() >= max_findings { + break; + } + } + if findings.len() >= max_findings { + break; } } + if findings.len() >= max_findings { + break; + } } // Sort Critical first diff --git a/src/engine/types.rs b/src/engine/types.rs index 326b71a..8d97fc9 100644 --- a/src/engine/types.rs +++ b/src/engine/types.rs @@ -2,9 +2,10 @@ use serde::{Deserialize, Serialize}; use std::fmt; /// Issue severity levels -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize, Default)] #[serde(rename_all = "lowercase")] pub enum Severity { + #[default] Critical, Major, Minor, diff --git a/src/index/extract.rs b/src/index/extract.rs index 8476615..0b013f2 100644 --- a/src/index/extract.rs +++ b/src/index/extract.rs @@ -419,29 +419,32 @@ fn detect_function_entry(line: &str, language: &str) -> Option { match language { "rs" => { - // pub fn name( or fn name( - let re = regex::Regex::new(r"(?:pub\s+)?(?:async\s+)?fn\s+(\w+)\s*[(<]").unwrap(); - re.captures(trimmed) + static RE: LazyLock = + LazyLock::new(|| Regex::new(r"(?:pub\s+)?(?:async\s+)?fn\s+(\w+)\s*[(<]").unwrap()); + RE.captures(trimmed) .map(|c| c.get(1).unwrap().as_str().to_string()) } "go" => { - let re = regex::Regex::new(r"func\s+(?:\([^)]+\)\s+)?(\w+)\s*\(").unwrap(); - re.captures(trimmed) + static RE: LazyLock = + LazyLock::new(|| Regex::new(r"func\s+(?:\([^)]+\)\s+)?(\w+)\s*\(").unwrap()); + RE.captures(trimmed) .map(|c| c.get(1).unwrap().as_str().to_string()) } "c" | "cpp" | "h" | "hpp" => { - let re = regex::Regex::new(r"[\w:*]+\s+(\w+)\s*\([^;]*\)\s*\{").unwrap(); - re.captures(trimmed) + static RE: LazyLock = + LazyLock::new(|| Regex::new(r"[\w:*]+\s+(\w+)\s*\([^;]*\)\s*\{").unwrap()); + RE.captures(trimmed) .map(|c| c.get(1).unwrap().as_str().to_string()) } "java" | "kt" => { - let re = regex::Regex::new(r"\b(\w+)\s*\([^)]*\)\s*\{").unwrap(); - re.captures(trimmed) + static RE: LazyLock = + LazyLock::new(|| Regex::new(r"\b(\w+)\s*\([^)]*\)\s*\{").unwrap()); + RE.captures(trimmed) .map(|c| c.get(1).unwrap().as_str().to_string()) } "py" => { - let re = regex::Regex::new(r"def\s+(\w+)").unwrap(); - re.captures(trimmed) + static RE: LazyLock = LazyLock::new(|| Regex::new(r"def\s+(\w+)").unwrap()); + RE.captures(trimmed) .map(|c| c.get(1).unwrap().as_str().to_string()) } _ => None, diff --git a/src/index/mod.rs b/src/index/mod.rs index 6fe76a5..d662ecd 100644 --- a/src/index/mod.rs +++ b/src/index/mod.rs @@ -243,18 +243,23 @@ pub fn prune_deleted(conn: &Connection, root: &Path) -> anyhow::Result { .filter_map(|r| r.ok()) .collect(); - for path in &paths { - let full = root.join(path); - if !full.exists() { - let tx = conn.unchecked_transaction()?; + // Batch all deletes in a single transaction instead of per-file + let to_prune: Vec<&String> = paths + .iter() + .filter(|path| !root.join(path).exists()) + .collect(); + + if !to_prune.is_empty() { + let tx = conn.unchecked_transaction()?; + for path in &to_prune { tx.execute( "DELETE FROM symbols WHERE file = ?1", rusqlite::params![path], )?; tx.execute("DELETE FROM files WHERE path = ?1", rusqlite::params![path])?; - tx.commit()?; - deleted += 1; } + tx.commit()?; + deleted = to_prune.len(); } if deleted > 0 { diff --git a/src/main.rs b/src/main.rs index 3190f8e..522320d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -842,29 +842,40 @@ async fn main() -> Result<()> { std::collections::HashSet::new(); // Strategy 1: Find symbols in changed files, then find callers that are in test files - for file in &changed { - // Get all symbols defined in this file - let symbols_in_file: Vec = { - let mut stmt = - conn.prepare("SELECT DISTINCT name FROM symbols WHERE file = ?1")?; - let rows = - stmt.query_map(rusqlite::params![file], |row| row.get::<_, String>(0))?; - rows.filter_map(|r| r.ok()).collect() - }; - - // For each symbol, find callers - for sym_name in &symbols_in_file { - let callers = index::graph::find_callers(&conn, sym_name, 100)?; - for caller in callers { - // Check if caller file looks like a test file - if patterns.iter().any(|p| caller.file.contains(p.as_str())) { - affected_tests.insert(caller.file.clone()); + // Batch: fetch all symbols for all changed files in a single query + let all_symbols: Vec = { + let placeholders: String = + changed.iter().map(|_| "?").collect::>().join(","); + let sql = + format!("SELECT DISTINCT name FROM symbols WHERE file IN ({placeholders})"); + let mut stmt = conn.prepare(&sql)?; + let params: Vec<&dyn rusqlite::types::ToSql> = changed + .iter() + .map(|f| f as &dyn rusqlite::types::ToSql) + .collect(); + let rows = stmt.query_map(params.as_slice(), |row| row.get::<_, String>(0))?; + rows.filter_map(|r| r.ok()).collect() + }; + + // Deduplicate symbols and resolve callers with a single set-based query + { + let mut seen_syms: std::collections::HashSet = + std::collections::HashSet::new(); + for sym_name in all_symbols { + if seen_syms.insert(sym_name.clone()) { + let callers = index::graph::find_callers(&conn, &sym_name, 100)?; + for caller in callers { + if patterns.iter().any(|p| caller.file.contains(p.as_str())) { + affected_tests.insert(caller.file.clone()); + } } } } } // Strategy 2: Direct test file name convention (mod_test.rs, foo_test.go) + // Prepare statement once before the loop + let mut stmt = conn.prepare("SELECT DISTINCT path FROM files WHERE path LIKE ?1")?; for file in &changed { // For Rust: src/foo.rs → tests/foo.rs or src/foo.rs → src/foo_test.rs let stem = file @@ -885,8 +896,6 @@ async fn main() -> Result<()> { format!("{stem}.spec.ts"), ]; for tp in &test_patterns { - let mut stmt = - conn.prepare("SELECT DISTINCT path FROM files WHERE path LIKE ?1")?; let pattern = format!("%{tp}"); let rows = stmt.query_map(rusqlite::params![pattern], |row| row.get::<_, String>(0))?; diff --git a/src/mcp/tools.rs b/src/mcp/tools.rs index 760b0c8..f8b6b66 100644 --- a/src/mcp/tools.rs +++ b/src/mcp/tools.rs @@ -2,6 +2,8 @@ //! //! Each handler takes JSON params and returns a ToolResult. +use std::sync::LazyLock; + use crate::engine::diff_parser; use crate::engine::profiles; use crate::engine::rules; @@ -10,6 +12,10 @@ use crate::engine::security_scanner; use super::protocol::{Tool, ToolResult}; +/// Shared Tokio runtime for MCP tool handlers — created once, reused across calls. +static MCP_RUNTIME: LazyLock = + LazyLock::new(|| tokio::runtime::Runtime::new().expect("Failed to create MCP tokio runtime")); + /// List all available MCP tools. pub fn list_tools() -> Vec { vec![ @@ -525,32 +531,44 @@ fn handle_find_affected_tests(params: &serde_json::Value) -> ToolResult { let patterns = ["test", "spec", "_test", "_spec"]; let mut affected: std::collections::HashSet = std::collections::HashSet::new(); - for file in &files { - // Strategy 1: call graph - let symbols_in_file: Vec = { - let mut stmt = match conn.prepare("SELECT DISTINCT name FROM symbols WHERE file = ?1") { - Ok(s) => s, - Err(e) => return ToolResult::error(format!("DB error: {e}")), - }; - let rows = match stmt.query_map(rusqlite::params![file], |row| row.get::<_, String>(0)) - { - Ok(r) => r, - Err(e) => return ToolResult::error(format!("DB error: {e}")), - }; - rows.filter_map(|r| r.ok()).collect() + // Batch fetch all symbols for all files in a single query + let all_symbols: Vec = { + let placeholders = files.iter().map(|_| "?").collect::>().join(","); + let sql = format!("SELECT DISTINCT name FROM symbols WHERE file IN ({placeholders})"); + let mut stmt = match conn.prepare(&sql) { + Ok(s) => s, + Err(e) => return ToolResult::error(format!("DB error: {e}")), }; + let params: Vec<&dyn rusqlite::types::ToSql> = files + .iter() + .map(|f| f as &dyn rusqlite::types::ToSql) + .collect(); + let rows = match stmt.query_map(params.as_slice(), |row| row.get::<_, String>(0)) { + Ok(r) => r, + Err(e) => return ToolResult::error(format!("DB error: {e}")), + }; + rows.filter_map(|r| r.ok()).collect() + }; - for sym_name in &symbols_in_file { - if let Ok(callers) = crate::index::graph::find_callers(&conn, sym_name, 100) { - for caller in callers { - if patterns.iter().any(|p| caller.file.contains(*p)) { - affected.insert(caller.file.clone()); + // Deduplicate and traverse call graph once + { + let mut seen_syms: std::collections::HashSet = std::collections::HashSet::new(); + for sym_name in &all_symbols { + if seen_syms.insert(sym_name.clone()) { + if let Ok(callers) = crate::index::graph::find_callers(&conn, sym_name, 100) { + for caller in callers { + if patterns.iter().any(|p| caller.file.contains(*p)) { + affected.insert(caller.file.clone()); + } } } } } + } - // Strategy 2: naming convention + // Strategy 2: naming convention — batch all test name candidates + let mut test_names: Vec = Vec::new(); + for file in &files { let stem = file .rsplit('/') .next() @@ -558,25 +576,39 @@ fn handle_find_affected_tests(params: &serde_json::Value) -> ToolResult { .rsplit('.') .next() .unwrap_or(""); - let test_names = [ + test_names.extend_from_slice(&[ format!("{stem}_test.rs"), format!("tests/{stem}.rs"), format!("{stem}_test.go"), format!("test_{stem}.py"), format!("{stem}.test.ts"), format!("{stem}.spec.ts"), - ]; - for tn in &test_names { - if let Ok(mut stmt) = conn.prepare("SELECT DISTINCT path FROM files WHERE path LIKE ?1") - { - let pattern = format!("%{tn}"); - if let Ok(rows) = - stmt.query_map(rusqlite::params![pattern], |row| row.get::<_, String>(0)) - { - for row in rows.map_while(Result::ok) { - affected.insert(row); - } - } + ]); + } + + // Query with a single LIKE batch + { + let sql = format!( + "SELECT DISTINCT path FROM files WHERE path LIKE '%' || ?1 OR {}", + test_names + .iter() + .enumerate() + .skip(1) + .map(|(i, _)| format!("path LIKE '%' || ?{}", i + 1)) + .collect::>() + .join(" OR ") + ); + let mut stmt = match conn.prepare(&sql) { + Ok(s) => s, + Err(e) => return ToolResult::error(format!("DB error: {e}")), + }; + let params: Vec<&dyn rusqlite::types::ToSql> = test_names + .iter() + .map(|t| t as &dyn rusqlite::types::ToSql) + .collect(); + if let Ok(rows) = stmt.query_map(params.as_slice(), |row| row.get::<_, String>(0)) { + for row in rows.map_while(Result::ok) { + affected.insert(row); } } } @@ -639,13 +671,8 @@ fn handle_review_diff(params: &serde_json::Value) -> ToolResult { } }; - // Run review synchronously - let rt = match tokio::runtime::Runtime::new() { - Ok(rt) => rt, - Err(e) => return ToolResult::error(format!("Failed to create runtime: {e}")), - }; - - let result = rt.block_on(crate::engine::review::review_diff_with_cache( + // Run review synchronously using the shared runtime + let result = MCP_RUNTIME.block_on(crate::engine::review::review_diff_with_cache( &config, &llm_config, diff,