diff --git a/.github/actions/cora-review/action.yml b/.github/actions/cora-review/action.yml index e0f97de..87575de 100644 --- a/.github/actions/cora-review/action.yml +++ b/.github/actions/cora-review/action.yml @@ -15,9 +15,9 @@ inputs: required: false default: 'latest' upload-sarif: - description: 'Upload SARIF to GitHub Code Scanning (default: false)' + description: 'Upload SARIF to GitHub Code Scanning (default: true). Set to "false" to disable.' required: false - default: 'false' + default: 'true' cora-api-key: description: 'LLM API key' required: true diff --git a/CHANGELOG.md b/CHANGELOG.md index 252ce96..76af9f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **Deterministic rule engine** — pre-LLM regex-based rules that always report findings (no LLM dismissal). Includes security (hardcoded URLs, secrets, TLS disabled, debug prints), SQL injection, and TODO markers (#116) +- **Custom rules via `.cora.yaml`** — define project-specific regex rules with severity, category, exclude patterns, and glob file matching +- **Unified diff parser** — parse git diff into structured `FileChunk`/`DiffHunk`/`DiffLine` with language detection for 70+ extensions +- **File bundling engine** — smart grouping by directory and language family with configurable character/file limits. Defers full parallel review to v0.5 (#115) +- **Cross-file context chain** — deterministic symbol extraction (imports, function calls, type references) for 5 languages (Rust, Python, JS, Go, Java) with token-budgeted context injection into LLM prompt (#114) +- **`BundlingConfig`** — `strategy`, `max_chars_per_group`, `max_files_per_group`, `coalesce_by_directory`, `coalesce_by_language` in `.cora.yaml` +- **`ContextConfig`** — `enabled`, `max_context_tokens`, `follow_depth`, `max_symbols` in `.cora.yaml` review section + +### Changed + +- **Review pipeline** — rules engine runs before LLM call, context chain enriches LLM prompt with cross-file dependencies + ## [0.3.0] - 2026-06-03 ### Added diff --git a/src/config/schema.rs b/src/config/schema.rs index f6daea8..75eddf4 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -1,8 +1,9 @@ +use crate::engine::Severity; +use crate::engine::bundling::types::BundlingConfig; +use crate::engine::rules::types::RulesConfig; use crate::error::CoraError; use serde::{Deserialize, Serialize}; -use crate::engine::Severity; - /// Runtime configuration — merged from defaults + .cora.yaml + CLI flags. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Config { @@ -38,6 +39,12 @@ pub struct Config { pub cache_ttl: u64, /// Static analysis context injection for reviews. pub static_analysis: StaticAnalysisConfig, + /// Rule engine configuration. + pub rules_config: RulesConfig, + /// Context chain configuration — cross-file dependency extraction. + pub context_chain: crate::engine::context::types::ContextConfig, + /// File bundling configuration for scan/review grouping. + pub bundling: BundlingConfig, } /// Provider configuration. @@ -113,6 +120,9 @@ impl Default for Config { timeout: 120, cache_ttl: 1440, // 24h in minutes static_analysis: StaticAnalysisConfig::default(), + rules_config: RulesConfig::default(), + context_chain: crate::engine::context::types::ContextConfig::default(), + bundling: BundlingConfig::default(), } } } @@ -145,6 +155,10 @@ pub struct CoraFile { pub scan: Option, #[serde(skip_serializing_if = "Option::is_none")] pub llm: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub rules_engine: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub bundling: Option, } #[derive(Debug, Clone, Serialize, Deserialize, Default)] @@ -195,6 +209,9 @@ pub struct ReviewSection { /// Static analysis context injection (e.g., clippy output). #[serde(skip_serializing_if = "Option::is_none")] pub static_analysis: Option, + /// Context chain configuration (cross-file dependency extraction). + #[serde(skip_serializing_if = "Option::is_none")] + pub context_chain: Option, } /// Static analysis configuration — inject linter/compiler output as review context. @@ -236,6 +253,36 @@ pub struct LlmSection { pub cache_ttl: Option, } +fn default_max_findings() -> usize { + 5 +} + +/// Rule engine configuration section for `.cora.yaml`. +#[derive(Debug, Clone, Serialize, Deserialize, Default)] +pub struct RulesSection { + #[serde(default, skip_serializing_if = "is_default")] + pub enabled: bool, + #[serde(default = "default_max_findings")] + pub max_findings: usize, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub custom: Vec, +} + +/// File bundling configuration section for `.cora.yaml`. +#[derive(Debug, Clone, Serialize, Deserialize, Default)] +pub struct BundlingSection { + #[serde(skip_serializing_if = "Option::is_none")] + pub max_chars_per_group: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub max_files_per_group: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub strategy: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub coalesce_by_directory: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub coalesce_by_language: Option, +} + impl CoraFile { pub fn from_str(content: &str) -> std::result::Result { serde_yaml_ng::from_str(content).map_err(|e| CoraError::ConfigParse(e.to_string())) @@ -324,6 +371,30 @@ impl CoraFile { config.cache_ttl = v; } } + if let Some(re) = &self.rules_engine { + config.rules_config.enabled = re.enabled; + config.rules_config.max_findings = re.max_findings; + if !re.custom.is_empty() { + config.rules_config.custom_rules = re.custom.clone(); + } + } + if let Some(b) = &self.bundling { + if let Some(v) = b.max_chars_per_group { + config.bundling.max_chars_per_group = v; + } + if let Some(v) = b.max_files_per_group { + config.bundling.max_files_per_group = v; + } + if let Some(v) = b.strategy { + config.bundling.strategy = v; + } + if let Some(v) = b.coalesce_by_directory { + config.bundling.coalesce_by_directory = v; + } + if let Some(v) = b.coalesce_by_language { + config.bundling.coalesce_by_language = v; + } + } } } @@ -656,6 +727,7 @@ review: system_prompt: None, system_prompt_file: None, static_analysis: None, + context_chain: None, }), ..Default::default() }; @@ -672,6 +744,7 @@ review: system_prompt: Some("Custom prompt here.".to_string()), system_prompt_file: None, static_analysis: None, + context_chain: None, }), ..Default::default() }; @@ -691,6 +764,7 @@ review: system_prompt: None, system_prompt_file: Some("prompts/review.md".to_string()), static_analysis: None, + context_chain: None, }), ..Default::default() }; @@ -927,4 +1001,138 @@ provider: assert_eq!(back.llm.as_ref().unwrap().temperature, Some(0.5)); assert_eq!(back.llm.as_ref().unwrap().max_tokens, Some(8192)); } + + // ─── BundlingSection parsing and merge ─── + + #[test] + fn config_default_bundling() { + let cfg = Config::default(); + assert_eq!(cfg.bundling.max_chars_per_group, 60_000); + assert_eq!(cfg.bundling.max_files_per_group, 20); + assert_eq!( + cfg.bundling.strategy, + crate::engine::bundling::GroupingStrategy::Smart + ); + assert!(cfg.bundling.coalesce_by_directory); + assert!(cfg.bundling.coalesce_by_language); + } + + #[test] + fn parse_bundling_section_full() { + let yaml = r" +bundling: + max_chars_per_group: 30000 + max_files_per_group: 10 + strategy: flat + coalesce_by_directory: false + coalesce_by_language: false +"; + let cora = CoraFile::from_str(yaml).unwrap(); + let b = cora.bundling.unwrap(); + assert_eq!(b.max_chars_per_group, Some(30_000)); + assert_eq!(b.max_files_per_group, Some(10)); + assert_eq!( + b.strategy, + Some(crate::engine::bundling::GroupingStrategy::Flat) + ); + assert_eq!(b.coalesce_by_directory, Some(false)); + assert_eq!(b.coalesce_by_language, Some(false)); + } + + #[test] + fn parse_bundling_section_partial() { + let yaml = r" +bundling: + max_chars_per_group: 40000 + strategy: flat +"; + let cora = CoraFile::from_str(yaml).unwrap(); + let b = cora.bundling.unwrap(); + assert_eq!(b.max_chars_per_group, Some(40_000)); + assert_eq!(b.max_files_per_group, None); + assert_eq!( + b.strategy, + Some(crate::engine::bundling::GroupingStrategy::Flat) + ); + assert_eq!(b.coalesce_by_directory, None); + assert_eq!(b.coalesce_by_language, None); + } + + #[test] + fn merge_bundling_all_fields() { + let mut cfg = Config::default(); + let cora = CoraFile { + bundling: Some(BundlingSection { + max_chars_per_group: Some(30_000), + max_files_per_group: Some(10), + strategy: Some(crate::engine::bundling::GroupingStrategy::Flat), + coalesce_by_directory: Some(false), + coalesce_by_language: Some(false), + }), + ..Default::default() + }; + cora.merge_into(&mut cfg); + assert_eq!(cfg.bundling.max_chars_per_group, 30_000); + assert_eq!(cfg.bundling.max_files_per_group, 10); + assert_eq!( + cfg.bundling.strategy, + crate::engine::bundling::GroupingStrategy::Flat + ); + assert!(!cfg.bundling.coalesce_by_directory); + assert!(!cfg.bundling.coalesce_by_language); + } + + #[test] + fn merge_bundling_partial() { + let mut cfg = Config::default(); + let cora = CoraFile { + bundling: Some(BundlingSection { + max_chars_per_group: Some(40_000), + max_files_per_group: None, + strategy: None, + coalesce_by_directory: None, + coalesce_by_language: Some(false), + }), + ..Default::default() + }; + cora.merge_into(&mut cfg); + assert_eq!(cfg.bundling.max_chars_per_group, 40_000); + assert_eq!(cfg.bundling.max_files_per_group, 20); // default unchanged + assert_eq!( + cfg.bundling.strategy, + crate::engine::bundling::GroupingStrategy::Smart + ); // default unchanged + assert!(cfg.bundling.coalesce_by_directory); // default unchanged + assert!(!cfg.bundling.coalesce_by_language); + } + + #[test] + fn merge_bundling_absent_leaves_defaults() { + let mut cfg = Config::default(); + let cora = CoraFile { + ..Default::default() + }; + cora.merge_into(&mut cfg); + assert_eq!(cfg.bundling.max_chars_per_group, 60_000); + assert_eq!(cfg.bundling.max_files_per_group, 20); + } + + #[test] + fn bundling_section_yaml_roundtrip() { + let section = BundlingSection { + max_chars_per_group: Some(50_000), + max_files_per_group: Some(15), + strategy: Some(crate::engine::bundling::GroupingStrategy::Smart), + coalesce_by_directory: Some(true), + coalesce_by_language: Some(false), + }; + let yaml = serde_yaml_ng::to_string(§ion).unwrap(); + let back: BundlingSection = serde_yaml_ng::from_str(&yaml).unwrap(); + assert_eq!(back.max_chars_per_group, Some(50_000)); + assert_eq!(back.max_files_per_group, Some(15)); + assert_eq!( + back.strategy, + Some(crate::engine::bundling::GroupingStrategy::Smart) + ); + } } diff --git a/src/engine/bundling/grouping.rs b/src/engine/bundling/grouping.rs new file mode 100644 index 0000000..01c8658 --- /dev/null +++ b/src/engine/bundling/grouping.rs @@ -0,0 +1,626 @@ +#[allow(dead_code)] +/// Grouping strategies for bundling files into LLM-friendly groups. +use std::collections::HashMap; + +use tracing::debug; + +use crate::engine::scanner::FileEntry; + +use super::types::{BundlingConfig, FileGroup, GroupingKey, GroupingStrategy, LanguageFamily}; + +/// Group files into `FileGroup`s based on the configuration. +/// +/// This is the main entry point for the bundling engine. It dispatches to +/// the appropriate strategy and returns a vector of groups. +pub fn group_files(files: &[FileEntry], config: &BundlingConfig) -> Vec { + if files.is_empty() { + return Vec::new(); + } + + match config.strategy { + GroupingStrategy::Smart => group_smart(files, config), + GroupingStrategy::Flat => group_flat(files, config), + } +} + +/// Smart grouping: coalesce by directory and language within character limits. +/// +/// Algorithm: +/// 1. Compute a `GroupingKey` (directory × language_family) for each file. +/// 2. Sort files by key so related files are adjacent. +/// 3. Greedily pack files into groups, preferring to keep the same key together. +/// 4. When a group is full (char or file limit), start a new one. +fn group_smart(files: &[FileEntry], config: &BundlingConfig) -> Vec { + debug!(total_files = files.len(), "smart grouping files"); + + let max_chars = config.max_chars_per_group; + let max_files = config.max_files_per_group; + + // Build grouping keys for each file + let mut keyed: Vec<(GroupingKey, &FileEntry)> = files + .iter() + .map(|f| { + let ext = f + .path + .rsplit_once('.') + .map(|(_, e)| e) + .filter(|e| !e.contains('/')); + let key = if config.coalesce_by_directory || config.coalesce_by_language { + GroupingKey::from_file(&f.path, ext) + } else { + GroupingKey { + directory: String::new(), + language: LanguageFamily::Other, + } + }; + (key, f) + }) + .collect(); + + // Sort by key to cluster related files + keyed.sort_by(|a, b| { + if config.coalesce_by_directory && config.coalesce_by_language { + a.0.directory + .cmp(&b.0.directory) + .then_with(|| a.0.language.cmp(&b.0.language)) + .then_with(|| a.1.path.cmp(&b.1.path)) + } else if config.coalesce_by_directory { + a.0.directory + .cmp(&b.0.directory) + .then_with(|| a.1.path.cmp(&b.1.path)) + } else if config.coalesce_by_language { + a.0.language + .cmp(&b.0.language) + .then_with(|| a.1.path.cmp(&b.1.path)) + } else { + a.1.path.cmp(&b.1.path) + } + }); + + // Greedy packing with coalescing preference + let mut groups: Vec = Vec::new(); + let mut current_group = FileGroup::new(); + let mut current_key: Option = None; + + for (key, file) in keyed { + let fits = current_group.would_fit(file, max_chars, max_files); + + // If file doesn't fit or we're switching to a very different key, consider + // starting a new group. But only split if we have enough files in the current + // group to justify it (avoid trivial groups). + let should_split = !fits + || (current_key.is_some() + && current_key.as_ref() != Some(&key) + && current_group.len() >= 2 + && would_exceed_threshold(¤t_group, file, max_chars)); + + if should_split && !current_group.is_empty() { + groups.push(std::mem::replace(&mut current_group, FileGroup::new())); + let _ = current_key.take(); + } + + current_group.push(file.clone()); + current_key = Some(key); + } + + if !current_group.is_empty() { + groups.push(current_group); + } + + debug!(groups = groups.len(), "smart grouping complete"); + + groups +} + +/// Check if adding a file would push the group past a "comfort threshold" +/// (75% of max_chars), suggesting it's a good split point. +fn would_exceed_threshold(group: &FileGroup, file: &FileEntry, max_chars: usize) -> bool { + let threshold = (max_chars as f64 * 0.75) as usize; + let cost = FileGroup::file_cost(file); + group.total_chars + cost > threshold +} + +/// Flat grouping: simple first-fit batching by character count. +/// +/// This is the legacy behavior — equivalent to the old `batch_files` function +/// in scanner.rs. Files are processed in order, packed greedily until limits +/// are hit, then a new group starts. +fn group_flat(files: &[FileEntry], config: &BundlingConfig) -> Vec { + debug!(total_files = files.len(), "flat grouping files"); + + let max_chars = config.max_chars_per_group; + let max_files = config.max_files_per_group; + + let mut groups: Vec = Vec::new(); + let mut current_group = FileGroup::new(); + + for file in files { + if !current_group.would_fit(file, max_chars, max_files) && !current_group.is_empty() { + groups.push(std::mem::replace(&mut current_group, FileGroup::new())); + } + current_group.push(file.clone()); + } + + if !current_group.is_empty() { + groups.push(current_group); + } + + debug!(groups = groups.len(), "flat grouping complete"); + + groups +} + +/// Group files by a specific key dimension (directory or language only). +/// +/// This is a helper for more fine-grained grouping when the user only wants +/// one dimension of coalescing. +pub fn group_by_dimension( + files: &[FileEntry], + config: &BundlingConfig, + dimension: GroupingDimension, +) -> Vec { + let max_chars = config.max_chars_per_group; + let max_files = config.max_files_per_group; + + // Bucket files by the chosen dimension + let mut buckets: HashMap> = HashMap::new(); + + for file in files { + let bucket_key = match dimension { + GroupingDimension::Directory => file + .path + .rsplit_once('/') + .or_else(|| file.path.rsplit_once('\\')) + .map(|(dir, _)| dir.to_string()) + .unwrap_or_else(|| ".".to_string()), + GroupingDimension::Language => file + .path + .rsplit_once('.') + .map(|(_, ext)| LanguageFamily::from_extension(ext).to_string()) + .unwrap_or_else(|| "other".to_string()), + }; + buckets.entry(bucket_key).or_default().push(file); + } + + // Sort bucket keys for deterministic output + let mut bucket_keys: Vec = buckets.keys().cloned().collect(); + bucket_keys.sort(); + + // Pack each bucket into groups respecting limits + let mut groups: Vec = Vec::new(); + + for key in bucket_keys { + let bucket_files = &buckets[&key]; + let mut current_group = FileGroup::new(); + + for file in bucket_files { + if !current_group.would_fit(file, max_chars, max_files) && !current_group.is_empty() { + groups.push(std::mem::replace(&mut current_group, FileGroup::new())); + } + current_group.push((*file).clone()); + } + + if !current_group.is_empty() { + groups.push(current_group); + } + } + + groups +} + +/// Dimension to group files by. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum GroupingDimension { + /// Group by parent directory. + Directory, + /// Group by language family. + Language, +} + +/// Merge small groups that could fit together. +/// +/// After initial grouping, some groups may be very small (1-2 files). +/// This post-processing step merges adjacent small groups if their +/// combined size stays within limits. +pub fn merge_small_groups( + mut groups: Vec, + config: &BundlingConfig, + min_group_size: usize, +) -> Vec { + if groups.len() <= 1 { + return groups; + } + + let max_chars = config.max_chars_per_group; + let max_files = config.max_files_per_group; + + let mut merged: Vec = Vec::new(); + let mut accumulator = groups.remove(0); + + for group in groups { + // Merge if both groups are small and combined fits + if accumulator.len() < min_group_size + && group.len() < min_group_size + && accumulator.len() + group.len() <= max_files + && accumulator.total_chars + group.total_chars <= max_chars + { + for file in group.files { + accumulator.push(file); + } + } else { + merged.push(std::mem::replace(&mut accumulator, group)); + } + } + + if !accumulator.is_empty() { + merged.push(accumulator); + } + + merged +} + +/// Collect statistics about the grouping result. +#[derive(Debug, Clone)] +pub struct GroupingStats { + /// Total number of groups. + pub group_count: usize, + /// Total number of files across all groups. + pub total_files: usize, + /// Total characters across all groups. + pub total_chars: usize, + /// Average files per group. + pub avg_files_per_group: f64, + /// Average characters per group. + pub avg_chars_per_group: f64, + /// Number of groups with only 1 file. + pub single_file_groups: usize, +} + +impl GroupingStats { + /// Compute stats from a list of groups. + pub fn from_groups(groups: &[FileGroup]) -> Self { + let group_count = groups.len(); + let total_files: usize = groups.iter().map(|g| g.len()).sum(); + let total_chars: usize = groups.iter().map(|g| g.total_chars).sum(); + let single_file_groups = groups.iter().filter(|g| g.len() == 1).count(); + + Self { + group_count, + total_files, + total_chars, + avg_files_per_group: if group_count > 0 { + total_files as f64 / group_count as f64 + } else { + 0.0 + }, + avg_chars_per_group: if group_count > 0 { + total_chars as f64 / group_count as f64 + } else { + 0.0 + }, + single_file_groups, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn make_file(path: &str, content_len: usize) -> FileEntry { + FileEntry { + path: path.to_string(), + content: "x".repeat(content_len), + lines: content_len, + } + } + + fn default_config() -> BundlingConfig { + BundlingConfig::default() + } + + // ─── group_files dispatch ─── + + #[test] + fn group_files_empty() { + let groups = group_files(&[], &default_config()); + assert!(groups.is_empty()); + } + + // ─── Flat grouping ─── + + #[test] + fn flat_grouping_basic() { + let files = vec![ + make_file("a.rs", 100), + make_file("b.rs", 100), + make_file("c.rs", 100), + ]; + let config = BundlingConfig { + max_chars_per_group: 250, // fits ~2 files per group + max_files_per_group: 100, + strategy: GroupingStrategy::Flat, + ..Default::default() + }; + + let groups = group_files(&files, &config); + assert!(groups.len() >= 2, "should split into at least 2 groups"); + assert_eq!( + groups.iter().map(|g| g.len()).sum::(), + 3, + "all files should be assigned" + ); + } + + #[test] + fn flat_grouping_all_fit() { + let files = vec![make_file("a.rs", 10), make_file("b.rs", 10)]; + let config = BundlingConfig { + strategy: GroupingStrategy::Flat, + ..Default::default() + }; + + let groups = group_files(&files, &config); + assert_eq!(groups.len(), 1); + assert_eq!(groups[0].len(), 2); + } + + #[test] + fn flat_grouping_preserves_order() { + let files = vec![ + make_file("a.rs", 10), + make_file("b.rs", 10), + make_file("c.rs", 10), + ]; + let config = BundlingConfig { + strategy: GroupingStrategy::Flat, + ..Default::default() + }; + + let groups = group_files(&files, &config); + assert_eq!(groups.len(), 1); + assert_eq!(groups[0].files[0].path, "a.rs"); + assert_eq!(groups[0].files[1].path, "b.rs"); + assert_eq!(groups[0].files[2].path, "c.rs"); + } + + #[test] + fn flat_grouping_max_files_limit() { + let files = vec![ + make_file("a.rs", 10), + make_file("b.rs", 10), + make_file("c.rs", 10), + make_file("d.rs", 10), + ]; + let config = BundlingConfig { + strategy: GroupingStrategy::Flat, + max_files_per_group: 2, + max_chars_per_group: 1_000_000, + ..Default::default() + }; + + let groups = group_files(&files, &config); + assert_eq!(groups.len(), 2); + assert_eq!(groups[0].len(), 2); + assert_eq!(groups[1].len(), 2); + } + + // ─── Smart grouping ─── + + #[test] + fn smart_grouping_clusters_same_directory() { + let files = vec![ + make_file("src/engine/mod.rs", 100), + make_file("src/engine/types.rs", 100), + make_file("src/config/mod.rs", 100), + make_file("tests/integration.rs", 100), + ]; + let config = BundlingConfig { + strategy: GroupingStrategy::Smart, + coalesce_by_directory: true, + coalesce_by_language: false, + ..Default::default() + }; + + let groups = group_files(&files, &config); + // Should cluster src/engine files together and src/config + tests may be separate + assert_eq!( + groups.iter().map(|g| g.len()).sum::(), + 4, + "all files assigned" + ); + } + + #[test] + fn smart_grouping_clusters_same_language() { + let files = vec![ + make_file("src/main.rs", 100), + make_file("lib/api.py", 100), + make_file("src/lib.rs", 100), + make_file("scripts/deploy.py", 100), + ]; + let config = BundlingConfig { + strategy: GroupingStrategy::Smart, + coalesce_by_directory: false, + coalesce_by_language: true, + ..Default::default() + }; + + let groups = group_files(&files, &config); + // Rust files should cluster together, Python files together + assert_eq!( + groups.iter().map(|g| g.len()).sum::(), + 4, + "all files assigned" + ); + } + + #[test] + fn smart_grouping_respects_char_limit() { + let files = vec![ + make_file("src/a.rs", 10_000), + make_file("src/b.rs", 10_000), + make_file("src/c.rs", 10_000), + ]; + let config = BundlingConfig { + strategy: GroupingStrategy::Smart, + max_chars_per_group: 15_000, + max_files_per_group: 100, + ..Default::default() + }; + + let groups = group_files(&files, &config); + assert!(groups.len() >= 2, "should split into at least 2 groups"); + } + + // ─── merge_small_groups ─── + + #[test] + fn merge_small_groups_basic() { + let g1 = { + let mut g = FileGroup::new(); + g.push(make_file("a.rs", 10)); + g + }; + let g2 = { + let mut g = FileGroup::new(); + g.push(make_file("b.rs", 10)); + g + }; + let g3 = { + let mut g = FileGroup::new(); + g.push(make_file("c.rs", 10)); + g + }; + + let merged = merge_small_groups(vec![g1, g2, g3], &default_config(), 2); + // All groups have 1 file (< min_group_size=2), so they should be merged + assert!(merged.len() <= 2, "should merge small groups"); + assert_eq!( + merged.iter().map(|g| g.len()).sum::(), + 3, + "no files lost" + ); + } + + #[test] + fn merge_small_groups_respects_limits() { + let g1 = { + let mut g = FileGroup::new(); + g.push(make_file("a.rs", 30_000)); + g + }; + let g2 = { + let mut g = FileGroup::new(); + g.push(make_file("b.rs", 30_000)); + g + }; + + let config = BundlingConfig { + max_chars_per_group: 50_000, + ..Default::default() + }; + let merged = merge_small_groups(vec![g1, g2], &config, 2); + // 30k + 30k = 60k > 50k, should not merge + assert_eq!(merged.len(), 2); + } + + // ─── group_by_dimension ─── + + #[test] + fn group_by_directory() { + let files = vec![ + make_file("src/engine/mod.rs", 10), + make_file("src/engine/types.rs", 10), + make_file("src/config/mod.rs", 10), + ]; + let groups = group_by_dimension(&files, &default_config(), GroupingDimension::Directory); + // Should have at least 2 groups: src/engine and src/config + assert!(groups.len() >= 2); + } + + #[test] + fn group_by_language() { + let files = vec![ + make_file("src/main.rs", 10), + make_file("src/app.py", 10), + make_file("src/lib.rs", 10), + ]; + let groups = group_by_dimension(&files, &default_config(), GroupingDimension::Language); + // Should have at least 2 groups: Rust and Python + assert!(groups.len() >= 2); + } + + // ─── GroupingStats ─── + + #[test] + fn grouping_stats_empty() { + let stats = GroupingStats::from_groups(&[]); + assert_eq!(stats.group_count, 0); + assert_eq!(stats.total_files, 0); + assert_eq!(stats.total_chars, 0); + assert_eq!(stats.single_file_groups, 0); + } + + #[test] + fn grouping_stats_computed() { + let g1 = { + let mut g = FileGroup::new(); + g.push(make_file("a.rs", 100)); + g.push(make_file("b.rs", 100)); + g + }; + let g2 = { + let mut g = FileGroup::new(); + g.push(make_file("c.rs", 50)); + g + }; + + let stats = GroupingStats::from_groups(&[g1, g2]); + assert_eq!(stats.group_count, 2); + assert_eq!(stats.total_files, 3); + assert_eq!(stats.single_file_groups, 1); + assert!((stats.avg_files_per_group - 1.5).abs() < 0.01); + } + + // ─── Smart grouping with both coalesce off ─── + + #[test] + fn smart_grouping_no_coalesce_is_like_flat() { + let files = vec![ + make_file("z.rs", 10), + make_file("a.rs", 10), + make_file("m.rs", 10), + ]; + let config = BundlingConfig { + strategy: GroupingStrategy::Smart, + coalesce_by_directory: false, + coalesce_by_language: false, + ..Default::default() + }; + + let groups = group_files(&files, &config); + assert_eq!(groups.len(), 1); + // Without coalescing, files are sorted by path + assert_eq!(groups[0].files[0].path, "a.rs"); + assert_eq!(groups[0].files[1].path, "m.rs"); + assert_eq!(groups[0].files[2].path, "z.rs"); + } + + // ─── Single large file ─── + + #[test] + fn single_large_file_gets_own_group() { + let files = vec![make_file("huge.rs", 200_000)]; + let config = BundlingConfig { + strategy: GroupingStrategy::Smart, + max_chars_per_group: 50_000, + max_files_per_group: 10, + ..Default::default() + }; + + let groups = group_files(&files, &config); + assert_eq!(groups.len(), 1); + assert_eq!(groups[0].len(), 1); + } +} diff --git a/src/engine/bundling/mod.rs b/src/engine/bundling/mod.rs new file mode 100644 index 0000000..9800401 --- /dev/null +++ b/src/engine/bundling/mod.rs @@ -0,0 +1,33 @@ +#[allow(dead_code, unused)] +/// File bundling engine — groups files into cohesive units for LLM review. +/// +/// Instead of arbitrary batching by character count alone, the bundling engine +/// groups related files together (by directory, language, or both) to provide +/// better review context to the LLM. +/// +/// # Configuration +/// +/// Controlled via `BundlingConfig` (on `Config`): +/// - `strategy`: `"smart"` (coalesce by directory/language) or `"flat"` (legacy) +/// - `max_chars_per_group`: character budget per group +/// - `max_files_per_group`: file count limit per group +/// - `coalesce_by_directory`: prefer keeping same-directory files together +/// - `coalesce_by_language`: prefer keeping same-language files together +/// +/// # Usage +/// +/// ```ignore +/// use crate::engine::bundling::{group_files, types::BundlingConfig}; +/// +/// let config = BundlingConfig::default(); +/// let groups = group_files(&files, &config); +/// for group in groups { +/// let content = crate::engine::scanner::format_batch_for_prompt(&group.files); +/// // send content to LLM +/// } +/// ``` +pub mod grouping; +pub mod types; + +#[allow(dead_code, unused)] +pub use types::GroupingStrategy; diff --git a/src/engine/bundling/types.rs b/src/engine/bundling/types.rs new file mode 100644 index 0000000..51ba395 --- /dev/null +++ b/src/engine/bundling/types.rs @@ -0,0 +1,450 @@ +#[allow(dead_code)] +/// Bundling types — configuration and data structures for file grouping. +use serde::{Deserialize, Serialize}; + +/// Runtime configuration for the file bundling engine (lives on `Config`). +/// +/// Controls how files are grouped before being sent to the LLM for review. +/// Smaller, cohesive groups give better review context than arbitrary batching +/// by character count alone. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct BundlingConfig { + /// Maximum number of characters per group (token budget). + /// Files are grouped until this limit is approached. + pub max_chars_per_group: usize, + /// Maximum number of files per group. + pub max_files_per_group: usize, + /// Preferred grouping strategy. + /// + /// - `"smart"` (default): group by directory, then by language, respecting + /// character limits. Keeps related files together. + /// - `"flat"`: simple first-fit batching by character count (legacy behavior, + /// equivalent to the old `batch_files` function). + #[serde(default = "default_grouping_strategy")] + pub strategy: GroupingStrategy, + /// Whether to keep files from the same directory together. + /// When `true`, files in the same directory are prioritized for the same group. + /// When `false`, groups may mix files from different directories. + #[serde(default = "default_true")] + pub coalesce_by_directory: bool, + /// Whether to group files by language (extension family). + /// When `true`, files with similar extensions (e.g., `.rs` + `.toml`) are + /// preferred for the same group. + #[serde(default = "default_true")] + pub coalesce_by_language: bool, +} + +impl Default for BundlingConfig { + fn default() -> Self { + Self { + max_chars_per_group: 60_000, + max_files_per_group: 20, + strategy: default_grouping_strategy(), + coalesce_by_directory: true, + coalesce_by_language: true, + } + } +} + +fn default_grouping_strategy() -> GroupingStrategy { + GroupingStrategy::Smart +} + +fn default_true() -> bool { + true +} + +/// Grouping strategy for file bundling. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum GroupingStrategy { + /// Smart grouping: coalesce by directory and language within limits. + Smart, + /// Flat batching: first-fit by character count (legacy). + Flat, +} + +impl std::fmt::Display for GroupingStrategy { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + GroupingStrategy::Smart => write!(f, "smart"), + GroupingStrategy::Flat => write!(f, "flat"), + } + } +} + +/// A group of files to be reviewed together in a single LLM call. +/// +/// Each group is a coherent unit — ideally files that are related by +/// directory, language, or feature area — kept within character and file +/// count limits. +#[derive(Debug, Clone)] +pub struct FileGroup { + /// Files in this group. + pub files: Vec, + /// Total character count (content + path headers + line numbers). + pub total_chars: usize, +} + +impl FileGroup { + /// Create a new empty group. + pub fn new() -> Self { + Self { + files: Vec::new(), + total_chars: 0, + } + } + + /// Estimated character size for a single file entry (content + header overhead). + pub fn file_cost(file: &crate::engine::scanner::FileEntry) -> usize { + file.content.len() + file.path.len() + 20 // header overhead + } + + /// Check if adding a file would exceed the limits. + pub fn would_fit( + &self, + file: &crate::engine::scanner::FileEntry, + max_chars: usize, + max_files: usize, + ) -> bool { + if self.files.len() >= max_files { + return false; + } + let cost = Self::file_cost(file); + // Allow first file even if it exceeds max_chars (single large file) + if self.files.is_empty() { + return true; + } + self.total_chars + cost <= max_chars + } + + /// Add a file to the group. + pub fn push(&mut self, file: crate::engine::scanner::FileEntry) { + self.total_chars += Self::file_cost(&file); + self.files.push(file); + } + + /// Number of files in the group. + pub fn len(&self) -> usize { + self.files.len() + } + + /// Whether the group is empty. + pub fn is_empty(&self) -> bool { + self.files.is_empty() + } +} + +/// Language family classification for grouping. +/// +/// Files with extensions in the same family are preferred to be grouped together, +/// as they often belong to the same project layer. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum LanguageFamily { + /// Rust (rs) + Rust, + /// JavaScript/TypeScript (js, ts, tsx, jsx, mjs, cjs) + JavaScript, + /// Python (py, pyi) + Python, + /// Go (go, mod, sum) + Go, + /// Java/JVM (java, kt, scala, gradle) + Jvm, + /// Web (html, css, scss, less, vue, svelte) + Web, + /// C/C++ (c, cpp, h, hpp, cc, cxx) + CFamily, + /// Shell (sh, bash, zsh, ps1) + Shell, + /// Config (yaml, yml, json, toml, ini) + Config, + /// Documentation (md, rst, txt, adoc) + Documentation, + /// Database (sql, graphql, proto) + Database, + /// Other / unclassified + Other, +} + +impl LanguageFamily { + /// Classify a file extension into a language family. + pub fn from_extension(ext: &str) -> Self { + match ext.to_lowercase().as_str() { + "rs" => LanguageFamily::Rust, + "js" | "ts" | "tsx" | "jsx" | "mjs" | "cjs" => LanguageFamily::JavaScript, + "py" | "pyi" => LanguageFamily::Python, + "go" | "mod" | "sum" => LanguageFamily::Go, + "java" | "kt" | "scala" | "gradle" => LanguageFamily::Jvm, + "html" | "css" | "scss" | "less" | "vue" | "svelte" => LanguageFamily::Web, + "c" | "cpp" | "h" | "hpp" | "cc" | "cxx" => LanguageFamily::CFamily, + "sh" | "bash" | "zsh" | "ps1" => LanguageFamily::Shell, + "yaml" | "yml" | "json" | "toml" | "ini" => LanguageFamily::Config, + "md" | "rst" | "txt" | "adoc" => LanguageFamily::Documentation, + "sql" | "graphql" | "proto" => LanguageFamily::Database, + _ => LanguageFamily::Other, + } + } +} + +impl std::fmt::Display for LanguageFamily { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + LanguageFamily::Rust => write!(f, "rust"), + LanguageFamily::JavaScript => write!(f, "javascript"), + LanguageFamily::Python => write!(f, "python"), + LanguageFamily::Go => write!(f, "go"), + LanguageFamily::Jvm => write!(f, "jvm"), + LanguageFamily::Web => write!(f, "web"), + LanguageFamily::CFamily => write!(f, "c_family"), + LanguageFamily::Shell => write!(f, "shell"), + LanguageFamily::Config => write!(f, "config"), + LanguageFamily::Documentation => write!(f, "documentation"), + LanguageFamily::Database => write!(f, "database"), + LanguageFamily::Other => write!(f, "other"), + } + } +} + +/// Key used for grouping files together. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct GroupingKey { + /// The parent directory of the file (relative to project root). + pub directory: String, + /// The language family of the file. + pub language: LanguageFamily, +} + +impl GroupingKey { + /// Create a grouping key from a file path and extension. + pub fn from_file(path: &str, extension: Option<&str>) -> Self { + let directory = path + .rsplit_once('/') + .or_else(|| path.rsplit_once('\\')) + .map(|(dir, _)| dir.to_string()) + .unwrap_or_else(|| ".".to_string()); + + let language = extension + .map(LanguageFamily::from_extension) + .unwrap_or(LanguageFamily::Other); + + Self { + directory, + language, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + // ─── BundlingConfig defaults ─── + + #[test] + fn bundling_config_default() { + let cfg = BundlingConfig::default(); + assert_eq!(cfg.max_chars_per_group, 60_000); + assert_eq!(cfg.max_files_per_group, 20); + assert_eq!(cfg.strategy, GroupingStrategy::Smart); + assert!(cfg.coalesce_by_directory); + assert!(cfg.coalesce_by_language); + } + + // ─── GroupingStrategy display ─── + + #[test] + fn grouping_strategy_display() { + assert_eq!(format!("{}", GroupingStrategy::Smart), "smart"); + assert_eq!(format!("{}", GroupingStrategy::Flat), "flat"); + } + + // ─── LanguageFamily classification ─── + + #[test] + fn language_family_rust() { + assert_eq!(LanguageFamily::from_extension("rs"), LanguageFamily::Rust); + assert_eq!( + LanguageFamily::from_extension("toml"), + LanguageFamily::Config + ); + } + + #[test] + fn language_family_javascript() { + assert_eq!( + LanguageFamily::from_extension("js"), + LanguageFamily::JavaScript + ); + assert_eq!( + LanguageFamily::from_extension("ts"), + LanguageFamily::JavaScript + ); + assert_eq!( + LanguageFamily::from_extension("tsx"), + LanguageFamily::JavaScript + ); + assert_eq!( + LanguageFamily::from_extension("jsx"), + LanguageFamily::JavaScript + ); + } + + #[test] + fn language_family_python() { + assert_eq!(LanguageFamily::from_extension("py"), LanguageFamily::Python); + } + + #[test] + fn language_family_other() { + assert_eq!(LanguageFamily::from_extension("xyz"), LanguageFamily::Other); + } + + #[test] + fn language_family_case_insensitive() { + assert_eq!(LanguageFamily::from_extension("RS"), LanguageFamily::Rust); + assert_eq!(LanguageFamily::from_extension("Py"), LanguageFamily::Python); + } + + // ─── GroupingKey ─── + + #[test] + fn grouping_key_from_file() { + let key = GroupingKey::from_file("src/engine/mod.rs", Some("rs")); + assert_eq!(key.directory, "src/engine"); + assert_eq!(key.language, LanguageFamily::Rust); + } + + #[test] + fn grouping_key_root_file() { + let key = GroupingKey::from_file("main.rs", Some("rs")); + assert_eq!(key.directory, "."); + assert_eq!(key.language, LanguageFamily::Rust); + } + + #[test] + fn grouping_key_no_extension() { + let key = GroupingKey::from_file("Makefile", None); + assert_eq!(key.directory, "."); + assert_eq!(key.language, LanguageFamily::Other); + } + + // ─── FileGroup ─── + + #[test] + fn file_group_new() { + let group = FileGroup::new(); + assert!(group.is_empty()); + assert_eq!(group.len(), 0); + assert_eq!(group.total_chars, 0); + } + + #[test] + fn file_group_push_and_fit() { + let mut group = FileGroup::new(); + let file = crate::engine::scanner::FileEntry { + path: "test.rs".to_string(), + content: "fn main() {}".to_string(), + lines: 1, + }; + + assert!(group.would_fit(&file, 1000, 10)); + group.push(file); + assert_eq!(group.len(), 1); + assert!(!group.is_empty()); + } + + #[test] + fn file_group_max_files_limit() { + let mut group = FileGroup::new(); + for i in 0..3 { + group.push(crate::engine::scanner::FileEntry { + path: format!("file_{i}.rs"), + content: "x".to_string(), + lines: 1, + }); + } + // 3 files already, max 3 + let new_file = crate::engine::scanner::FileEntry { + path: "file_4.rs".to_string(), + content: "x".to_string(), + lines: 1, + }; + assert!(!group.would_fit(&new_file, 1_000_000, 3)); + } + + #[test] + fn file_group_max_chars_limit() { + let mut group = FileGroup::new(); + group.push(crate::engine::scanner::FileEntry { + path: "big.rs".to_string(), + content: "x".repeat(500), + lines: 1, + }); + let new_file = crate::engine::scanner::FileEntry { + path: "small.rs".to_string(), + content: "x".repeat(500), + lines: 1, + }; + // First file always fits, second should not if we set tight limit + // Total would be ~500 + path + 20 + 500 + path + 20 ≈ 1060 + assert!(!group.would_fit(&new_file, 800, 100)); + } + + #[test] + fn file_group_first_file_always_fits() { + let group = FileGroup::new(); + let huge_file = crate::engine::scanner::FileEntry { + path: "huge.rs".to_string(), + content: "x".repeat(1_000_000), + lines: 1000, + }; + // Even with tiny char limit, first file should fit + assert!(group.would_fit(&huge_file, 100, 10)); + } + + #[test] + fn file_cost_calculation() { + let file = crate::engine::scanner::FileEntry { + path: "test.rs".to_string(), + content: "hello world".to_string(), + lines: 1, + }; + let cost = FileGroup::file_cost(&file); + // "hello world".len() = 11 + "test.rs".len() = 7 + 20 overhead = 38 + assert_eq!(cost, 38); + } + + // ─── GroupingKey equality ─── + + #[test] + fn grouping_key_equality() { + let k1 = GroupingKey::from_file("src/main.rs", Some("rs")); + let k2 = GroupingKey::from_file("src/lib.rs", Some("rs")); + let k3 = GroupingKey::from_file("src/main.rs", Some("py")); + + assert_eq!(k1, k2); // same directory + language + assert_ne!(k1, k3); // same directory, different language + } + + // ─── BundlingConfig serde round-trip ─── + + #[test] + fn bundling_config_serde_roundtrip() { + let cfg = BundlingConfig { + max_chars_per_group: 30_000, + max_files_per_group: 10, + strategy: GroupingStrategy::Flat, + coalesce_by_directory: false, + coalesce_by_language: false, + }; + let json = serde_json::to_string(&cfg).unwrap(); + let back: BundlingConfig = serde_json::from_str(&json).unwrap(); + assert_eq!(back.max_chars_per_group, 30_000); + assert_eq!(back.max_files_per_group, 10); + assert_eq!(back.strategy, GroupingStrategy::Flat); + assert!(!back.coalesce_by_directory); + assert!(!back.coalesce_by_language); + } +} diff --git a/src/engine/context/extraction.rs b/src/engine/context/extraction.rs new file mode 100644 index 0000000..53f0fb5 --- /dev/null +++ b/src/engine/context/extraction.rs @@ -0,0 +1,664 @@ +//! Symbol extraction from source code lines. +//! +//! Uses language-aware regex patterns to extract function calls, type +//! references, and import statements from changed diff lines. No full +//! AST parsing — regex is sufficient for the deterministic context chain +//! and avoids pulling in heavy dependencies like `syn`. + +use std::collections::HashSet; + +use regex::Regex; +use std::sync::LazyLock; +use tracing::debug; + +use super::types::{ExtractedSymbol, SymbolKind}; + +// --- Regex patterns by language --- + +/// Rust: `use crate::foo::bar` or `use super::baz` or `mod foo` +static RE_RUST_IMPORT: LazyLock = + LazyLock::new(|| Regex::new(r"\buse\s+(crate|super|self)::([\w:]+)").unwrap()); + +/// Rust: function call — identifier followed by `(`, possibly preceded by `::` or `.` +static RE_RUST_FN_CALL: LazyLock = + LazyLock::new(|| Regex::new(r"(?:\w+::)?\w+(?:::\w+)*\s*\(").unwrap()); + +/// Rust: type-like references — `Type`, `Option`, `Vec`, after `: `, `<`, or `as` +static RE_RUST_TYPE: LazyLock = + LazyLock::new(|| Regex::new(r"(?:(?::\s)|(?:<)|(?:as\s)|(?:->\s))([A-Z]\w+)").unwrap()); + +/// Python: `import foo` or `from foo import bar` +static RE_PY_IMPORT: LazyLock = + LazyLock::new(|| Regex::new(r"\b(?:from|import)\s+([\w.]+)").unwrap()); + +/// Python: function call — `foo(` or `obj.method(` +static RE_PY_FN_CALL: LazyLock = LazyLock::new(|| Regex::new(r"\b(\w+)\s*\(").unwrap()); + +/// Python: type annotation — `: SomeType` or `-> ReturnType` +static RE_PY_TYPE: LazyLock = + LazyLock::new(|| Regex::new(r"(?::\s*[A-Z]\w*|->\s*[A-Z]\w*)").unwrap()); + +/// JS/TS: `import ... from '...'`, `require('...')`, or dynamic `import('...')` +static RE_JS_IMPORT: LazyLock = LazyLock::new(|| { + Regex::new(r#"(?:import\s+.*?\s+from\s*['"]([^'"]+)['"]|require\s*\(\s*['"]([^'"]+)['"]|import\s*\(\s*['"]([^'"]+)['"])"#).unwrap() +}); + +/// JS/TS: function call — `foo(` or `obj.method(` +static RE_JS_FN_CALL: LazyLock = LazyLock::new(|| Regex::new(r"\b(\w+)\s*\(").unwrap()); + +/// Go: `import "foo"` or `import foo "bar"` +static RE_GO_IMPORT: LazyLock = + LazyLock::new(|| Regex::new(r#"\bimport\s+[\w.]*\s*["']([^"']+)["']"#).unwrap()); + +/// Go: function call — `foo(` or `pkg.Func(` +static RE_GO_FN_CALL: LazyLock = + LazyLock::new(|| Regex::new(r"\b([A-Z]\w*\.\w+|\w+)\s*\(").unwrap()); + +/// Go: type reference — `SomeType{` or `var x SomeType` or `: SomeType` +static RE_GO_TYPE: LazyLock = + LazyLock::new(|| Regex::new(r"(?:\b([A-Z]\w+)\s*\{|:\s*([A-Z]\w+))").unwrap()); + +/// Java: `import foo.bar.*` +static RE_JAVA_IMPORT: LazyLock = + LazyLock::new(|| Regex::new(r"\bimport\s+([\w.]+)").unwrap()); + +/// Java: method call — `foo(` or `obj.method(` +static RE_JAVA_FN_CALL: LazyLock = LazyLock::new(|| Regex::new(r"\b(\w+)\s*\(").unwrap()); + +/// Java: type reference — `SomeType ` or `` +static RE_JAVA_TYPE: LazyLock = + LazyLock::new(|| Regex::new(r"\b([A-Z]\w+)\s*[<{( ]").unwrap()); + +/// Maximum number of symbols to extract per file to prevent runaway extraction. +const MAX_SYMBOLS_PER_FILE: usize = 50; + +/// Extract symbols from a single line of code for a given language. +fn extract_symbols_from_line(line: &str, language: &str) -> Vec { + let mut symbols = Vec::new(); + let mut seen = HashSet::new(); + + let add_unique = |syms: &mut Vec, seen: &mut HashSet, s: SymbolKind| { + let key = format!("{s}"); + if seen.insert(key) { + syms.push(s); + } + }; + + match language { + "rs" => { + // Imports (highest value for resolution) + for cap in RE_RUST_IMPORT.captures_iter(line) { + add_unique( + &mut symbols, + &mut seen, + SymbolKind::Import(cap.get(2).unwrap().as_str().to_string()), + ); + } + // Function calls + for cap in RE_RUST_FN_CALL.captures_iter(line) { + let raw = cap.get(0).unwrap().as_str().trim_end_matches('(').trim(); + // Skip macros like `println!`, `vec!`, `format!`, `debug!`, etc. + if raw.ends_with('!') { + continue; + } + // Skip common keywords that look like function calls + if matches!( + raw, + "if" | "while" | "for" | "loop" | "match" | "return" | "break" + ) { + continue; + } + add_unique( + &mut symbols, + &mut seen, + SymbolKind::FunctionCall(raw.to_string()), + ); + } + // Type references + for cap in RE_RUST_TYPE.captures_iter(line) { + if let Some(type_name) = cap.get(1) { + let name = type_name.as_str().to_string(); + add_unique(&mut symbols, &mut seen, SymbolKind::TypeRef(name)); + } + } + } + "py" | "pyi" => { + for cap in RE_PY_IMPORT.captures_iter(line) { + add_unique( + &mut symbols, + &mut seen, + SymbolKind::Import(cap.get(1).unwrap().as_str().to_string()), + ); + } + for cap in RE_PY_FN_CALL.captures_iter(line) { + let name = cap.get(1).unwrap().as_str().to_string(); + if matches!( + name.as_str(), + "if" | "while" | "for" | "print" | "range" | "len" + ) { + continue; + } + add_unique(&mut symbols, &mut seen, SymbolKind::FunctionCall(name)); + } + for cap in RE_PY_TYPE.captures_iter(line) { + let full = cap.get(0).unwrap().as_str(); + // Extract the type name after `:` or `->` + let name = full.trim_start_matches(':').trim_start_matches('-').trim(); + if !name.is_empty() && name.starts_with(char::is_uppercase) { + add_unique( + &mut symbols, + &mut seen, + SymbolKind::TypeRef(name.to_string()), + ); + } + } + } + "ts" | "tsx" | "js" | "jsx" | "mjs" | "cjs" => { + for cap in RE_JS_IMPORT.captures_iter(line) { + // The regex has 3 capture groups (one per alternative). Take the first non-None. + let path = cap + .get(1) + .or_else(|| cap.get(2)) + .or_else(|| cap.get(3)) + .unwrap() + .as_str() + .to_string(); + add_unique(&mut symbols, &mut seen, SymbolKind::Import(path)); + } + for cap in RE_JS_FN_CALL.captures_iter(line) { + let name = cap.get(1).unwrap().as_str().to_string(); + if matches!(name.as_str(), "if" | "while" | "for" | "require" | "import") { + continue; + } + add_unique(&mut symbols, &mut seen, SymbolKind::FunctionCall(name)); + } + } + "go" => { + for cap in RE_GO_IMPORT.captures_iter(line) { + add_unique( + &mut symbols, + &mut seen, + SymbolKind::Import(cap.get(1).unwrap().as_str().to_string()), + ); + } + for cap in RE_GO_FN_CALL.captures_iter(line) { + let name = cap.get(1).unwrap().as_str().to_string(); + if matches!(name.as_str(), "if" | "for" | "range" | "defer" | "go") { + continue; + } + add_unique(&mut symbols, &mut seen, SymbolKind::FunctionCall(name)); + } + for cap in RE_GO_TYPE.captures_iter(line) { + if let Some(type_name) = cap.get(1).or_else(|| cap.get(2)) { + add_unique( + &mut symbols, + &mut seen, + SymbolKind::TypeRef(type_name.as_str().to_string()), + ); + } + } + } + "java" | "kt" | "kts" => { + for cap in RE_JAVA_IMPORT.captures_iter(line) { + add_unique( + &mut symbols, + &mut seen, + SymbolKind::Import(cap.get(1).unwrap().as_str().to_string()), + ); + } + for cap in RE_JAVA_FN_CALL.captures_iter(line) { + let name = cap.get(1).unwrap().as_str().to_string(); + if matches!( + name.as_str(), + "if" | "while" | "for" | "switch" | "return" | "new" | "instanceof" + ) { + continue; + } + add_unique(&mut symbols, &mut seen, SymbolKind::FunctionCall(name)); + } + for cap in RE_JAVA_TYPE.captures_iter(line) { + if let Some(type_name) = cap.get(1) { + let name = type_name.as_str().to_string(); + // Skip common non-user types + if matches!( + name.as_str(), + "String" | "Integer" | "Boolean" | "List" | "Map" + ) { + continue; + } + add_unique(&mut symbols, &mut seen, SymbolKind::TypeRef(name)); + } + } + } + _ => { + // Fallback: basic function call detection for any language + for cap in RE_PY_FN_CALL.captures_iter(line) { + let name = cap.get(1).unwrap().as_str().to_string(); + if name.len() > 2 { + add_unique(&mut symbols, &mut seen, SymbolKind::FunctionCall(name)); + } + } + } + } + + symbols +} + +/// Extract all symbols from the added lines of parsed diff chunks. +/// +/// Returns a list of `ExtractedSymbol` entries, one per symbol found. +/// Duplicates across lines are deduplicated (same kind + same name + same file). +pub fn extract_symbols_from_diff( + chunks: &[crate::engine::diff_parser::FileChunk], +) -> Vec { + let mut all_symbols = Vec::new(); + let mut seen: HashSet<(String, String)> = HashSet::new(); // (symbol key, file) + + for file in chunks { + if file.is_binary || file.is_deleted { + continue; + } + + let file_path = file + .new_path + .as_deref() + .or(file.old_path.as_deref()) + .unwrap_or("unknown"); + + let mut file_symbol_count = 0; + + for hunk in &file.chunks { + for line in &hunk.lines { + // Only extract from added lines + if line.line_type != crate::engine::diff_parser::DiffLineType::Add { + continue; + } + + 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 { + file: file_path.to_string(), + line: line.new_line_no.unwrap_or(0), + kind: sym, + raw: line.content.clone(), + }); + file_symbol_count += 1; + } + } + } + } + + debug!( + file = file_path, + symbols = file_symbol_count, + "extracted symbols from file" + ); + } + + debug!( + total = all_symbols.len(), + "total symbols extracted from diff" + ); + all_symbols +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::engine::diff_parser::{DiffHunk, DiffLine, DiffLineType, FileChunk, parse_diff}; + + fn make_file_chunk(path: &str, lang: &str, added_lines: &[(&str, u32)]) -> FileChunk { + FileChunk { + old_path: Some(path.to_string()), + new_path: Some(path.to_string()), + language: lang.to_string(), + chunks: vec![DiffHunk { + old_start: 1, + old_count: 0, + new_start: 1, + new_count: added_lines.len() as u32, + header: String::new(), + lines: added_lines + .iter() + .map(|(text, line_no)| DiffLine { + line_type: DiffLineType::Add, + content: text.to_string(), + old_line_no: None, + new_line_no: Some(*line_no), + }) + .collect(), + }], + is_binary: false, + is_deleted: false, + is_new: false, + } + } + + // --- Rust extraction --- + + #[test] + fn extract_rust_function_calls() { + let chunks = vec![make_file_chunk( + "src/main.rs", + "rs", + &[("let result = validate_token(tok);", 5)], + )]; + let symbols = extract_symbols_from_diff(&chunks); + let fn_calls: Vec<_> = symbols + .iter() + .filter( + |s| matches!(&s.kind, SymbolKind::FunctionCall(name) if name == "validate_token"), + ) + .collect(); + assert!( + !fn_calls.is_empty(), + "should extract validate_token function call from Rust" + ); + } + + #[test] + fn extract_rust_imports() { + let chunks = vec![make_file_chunk( + "src/engine/mod.rs", + "rs", + &[("use crate::engine::scanner;", 1)], + )]; + let symbols = extract_symbols_from_diff(&chunks); + let imports: Vec<_> = symbols + .iter() + .filter(|s| matches!(&s.kind, SymbolKind::Import(p) if p == "engine::scanner")) + .collect(); + assert!( + !imports.is_empty(), + "should extract use crate::engine::scanner" + ); + } + + #[test] + fn extract_rust_type_refs() { + let chunks = vec![make_file_chunk( + "src/config.rs", + "rs", + &[("let config: CryptoConfig = load();", 10)], + )]; + let symbols = extract_symbols_from_diff(&chunks); + let types: Vec<_> = symbols + .iter() + .filter(|s| matches!(&s.kind, SymbolKind::TypeRef(t) if t == "CryptoConfig")) + .collect(); + assert!( + !types.is_empty(), + "should extract CryptoConfig type reference" + ); + } + + #[test] + fn rust_skips_macros() { + let chunks = vec![make_file_chunk( + "src/main.rs", + "rs", + &[ + ("println!(\"hello\");", 1), + ("debug!(\"value: {}\", x);", 2), + ], + )]; + let symbols = extract_symbols_from_diff(&chunks); + let fn_calls: Vec<_> = symbols + .iter() + .filter(|s| matches!(&s.kind, SymbolKind::FunctionCall(_))) + .collect(); + assert!( + fn_calls.is_empty(), + "should skip Rust macros (println!, debug!, etc.)" + ); + } + + #[test] + fn rust_skips_keywords() { + let chunks = vec![make_file_chunk( + "src/main.rs", + "rs", + &[("if condition { return; }", 1)], + )]; + let symbols = extract_symbols_from_diff(&chunks); + let fn_calls: Vec<_> = symbols + .iter() + .filter(|s| matches!(&s.kind, SymbolKind::FunctionCall(name) if name == "if" || name == "return")) + .collect(); + assert!(fn_calls.is_empty(), "should skip Rust keyword-like calls"); + } + + // --- Python extraction --- + + #[test] + fn extract_python_imports() { + let chunks = vec![make_file_chunk( + "app/auth.py", + "py", + &[("from auth import validate_token", 1)], + )]; + let symbols = extract_symbols_from_diff(&chunks); + let imports: Vec<_> = symbols + .iter() + .filter(|s| matches!(&s.kind, SymbolKind::Import(p) if p == "auth")) + .collect(); + assert!(!imports.is_empty(), "should extract Python from/import"); + } + + #[test] + fn extract_python_function_calls() { + let chunks = vec![make_file_chunk( + "app/auth.py", + "py", + &[("result = validate_token(tok)", 5)], + )]; + let symbols = extract_symbols_from_diff(&chunks); + let fn_calls: Vec<_> = symbols + .iter() + .filter( + |s| matches!(&s.kind, SymbolKind::FunctionCall(name) if name == "validate_token"), + ) + .collect(); + assert!( + !fn_calls.is_empty(), + "should extract validate_token from Python" + ); + } + + // --- JavaScript/TypeScript extraction --- + + #[test] + fn extract_js_imports() { + let chunks = vec![make_file_chunk( + "src/api.ts", + "ts", + &[("import { validate } from './auth';", 1)], + )]; + let symbols = extract_symbols_from_diff(&chunks); + let imports: Vec<_> = symbols + .iter() + .filter(|s| matches!(&s.kind, SymbolKind::Import(p) if p == "./auth")) + .collect(); + assert!(!imports.is_empty(), "should extract JS/TS import path"); + } + + // --- Go extraction --- + + #[test] + fn extract_go_imports() { + let chunks = vec![make_file_chunk( + "main.go", + "go", + &[("import \"net/http\"", 3)], + )]; + let symbols = extract_symbols_from_diff(&chunks); + let imports: Vec<_> = symbols + .iter() + .filter(|s| matches!(&s.kind, SymbolKind::Import(p) if p == "net/http")) + .collect(); + assert!(!imports.is_empty(), "should extract Go import"); + } + + // --- Edge cases --- + + #[test] + fn skips_binary_files() { + let chunks = vec![FileChunk { + old_path: Some("image.png".to_string()), + new_path: Some("image.png".to_string()), + language: "image".to_string(), + chunks: vec![], + is_binary: true, + is_deleted: false, + is_new: false, + }]; + let symbols = extract_symbols_from_diff(&chunks); + assert!(symbols.is_empty(), "should skip binary files"); + } + + #[test] + fn skips_deleted_files() { + let chunks = vec![FileChunk { + old_path: Some("old.rs".to_string()), + new_path: None, + language: "rs".to_string(), + chunks: vec![], + is_binary: false, + is_deleted: true, + is_new: false, + }]; + let symbols = extract_symbols_from_diff(&chunks); + assert!(symbols.is_empty(), "should skip deleted files"); + } + + #[test] + fn skips_context_and_removed_lines() { + let chunks = vec![FileChunk { + old_path: Some("src/lib.rs".to_string()), + new_path: Some("src/lib.rs".to_string()), + language: "rs".to_string(), + chunks: vec![DiffHunk { + old_start: 1, + old_count: 2, + new_start: 1, + new_count: 1, + header: String::new(), + lines: vec![ + DiffLine { + line_type: DiffLineType::Context, + content: "fn old() {}".to_string(), + old_line_no: Some(1), + new_line_no: Some(1), + }, + DiffLine { + line_type: DiffLineType::Remove, + content: "fn removed() {}".to_string(), + old_line_no: Some(2), + new_line_no: None, + }, + ], + }], + is_binary: false, + is_deleted: false, + is_new: false, + }]; + let symbols = extract_symbols_from_diff(&chunks); + assert!(symbols.is_empty(), "should only extract from added lines"); + } + + #[test] + fn deduplicates_across_lines() { + let chunks = vec![make_file_chunk( + "src/main.rs", + "rs", + &[ + ("let a = foo();", 1), + ("let b = foo();", 2), + ("let c = bar();", 3), + ], + )]; + let symbols = extract_symbols_from_diff(&chunks); + let foo_count = symbols + .iter() + .filter(|s| matches!(&s.kind, SymbolKind::FunctionCall(n) if n == "foo")) + .count(); + assert_eq!( + foo_count, 1, + "should deduplicate same symbol across lines in same file" + ); + } + + #[test] + fn respects_max_symbols_per_file() { + let lines: Vec<(&str, u32)> = (0..60u32) + .map(|i| { + let s = format!("let x{i} = func{i}();"); + // Leak to get &'static str + let leaked: &'static str = Box::leak(s.into_boxed_str()); + (leaked, i + 1) + }) + .collect(); + let chunks = vec![make_file_chunk("src/big.rs", "rs", &lines)]; + let symbols = extract_symbols_from_diff(&chunks); + assert!( + symbols.len() <= MAX_SYMBOLS_PER_FILE, + "should cap at MAX_SYMBOLS_PER_FILE" + ); + } + + #[test] + fn unknown_language_fallback() { + let chunks = vec![make_file_chunk( + "README.md", + "md", + &[("see foo() for details", 1)], + )]; + let symbols = extract_symbols_from_diff(&chunks); + // Fallback should at least extract function calls + let fn_calls: Vec<_> = symbols + .iter() + .filter(|s| matches!(&s.kind, SymbolKind::FunctionCall(n) if n == "foo")) + .collect(); + assert!( + !fn_calls.is_empty(), + "fallback should extract function calls for unknown languages" + ); + } + + #[test] + fn full_diff_extraction() { + let diff = r#"diff --git a/src/auth.rs b/src/auth.rs +--- a/src/auth.rs ++++ b/src/auth.rs +@@ -10,6 +10,8 @@ + use crate::config::CryptoConfig; ++use crate::engine::scanner; + + pub fn authenticate(req: &Request) -> Result { ++ let config = CryptoConfig::load(); ++ let valid = validate_token(&req.token, &config); ++ scanner::scan_file(&req.path) + } +"#; + let chunks = parse_diff(diff); + let symbols = extract_symbols_from_diff(&chunks); + // Should find imports, function calls, and type refs + assert!(!symbols.is_empty(), "should extract symbols from full diff"); + let has_import = symbols + .iter() + .any(|s| matches!(&s.kind, SymbolKind::Import(_))); + let has_fn_call = symbols + .iter() + .any(|s| matches!(&s.kind, SymbolKind::FunctionCall(_))); + assert!(has_import, "should find import symbols"); + assert!(has_fn_call, "should find function call symbols"); + } +} diff --git a/src/engine/context/mod.rs b/src/engine/context/mod.rs new file mode 100644 index 0000000..ff87282 --- /dev/null +++ b/src/engine/context/mod.rs @@ -0,0 +1,106 @@ +//! Context chain — deterministic cross-file dependency extraction. +//! +//! Enriches LLM review prompts with function definitions, type definitions, +//! and test coverage information extracted from the changed code, without +//! using any LLM tokens. +//! +//! ## Architecture +//! +//! 1. **Extraction** (`extraction.rs`): Extract function calls, type refs, +//! and imports from changed diff lines using language-aware regex. +//! +//! 2. **Resolution** (`resolver.rs`): Resolve extracted symbols to concrete +//! file locations (e.g., `use crate::auth::validate` → `src/auth/validate.rs`). +//! +//! 3. **Assembly**: Read file content under a token budget, assemble into a +//! formatted string for prompt injection. +//! +//! ## Usage +//! +//! ```rust,ignore +//! let chain = context::build_context_chain( +//! &extracted_symbols, +//! &config.context_chain, +//! &project_root, +//! &config.ignore.files, +//! ); +//! if !chain.text.is_empty() { +//! // Inject chain.text into the review prompt +//! } +//! ``` + +pub mod extraction; +pub mod resolver; +pub mod types; + +use std::path::Path; + +use crate::engine::diff_parser::FileChunk; +use types::{ContextChain, ContextConfig}; + +/// Build the full context chain from parsed diff chunks. +/// +/// Convenience entry point that combines extraction and resolution. +/// Returns the assembled context chain ready for prompt injection. +pub fn build_context_chain( + chunks: &[FileChunk], + config: &ContextConfig, + project_root: &Path, + ignore_patterns: &[String], +) -> ContextChain { + // Step 1: Extract symbols from changed lines + let symbols = extraction::extract_symbols_from_diff(chunks); + + if symbols.is_empty() || !config.enabled { + return ContextChain::default(); + } + + // Step 2: Resolve and assemble under budget + resolver::build_context_chain(&symbols, config, project_root, ignore_patterns) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn default_config() -> ContextConfig { + ContextConfig::default() + } + + #[test] + fn empty_chunks_returns_empty() { + let chain = build_context_chain(&[], &default_config(), Path::new("/tmp"), &[]); + assert!(chain.text.is_empty()); + } + + #[test] + fn disabled_returns_empty() { + let config = ContextConfig { + enabled: false, + ..default_config() + }; + let chunks = crate::engine::diff_parser::parse_diff( + "diff --git a/src/main.rs b/src/main.rs\n--- a/src/main.rs\n+++ b/src/main.rs\n@@ -1,1 +1,2 @@\n fn main() {\n+ foo();\n }\n", + ); + let chain = build_context_chain(&chunks, &config, Path::new("/tmp"), &[]); + assert!(chain.text.is_empty()); + } + + #[test] + fn extracts_from_real_diff() { + let diff = r#"diff --git a/src/main.rs b/src/main.rs +--- a/src/main.rs ++++ b/src/main.rs +@@ -5,6 +5,8 @@ + fn main() { + println!("hello"); ++ let config = load_config(); ++ authenticate(&config); + } +"#; + let chunks = crate::engine::diff_parser::parse_diff(diff); + let chain = build_context_chain(&chunks, &default_config(), Path::new("/nonexistent"), &[]); + // Should extract symbols but not resolve (no files exist) + assert_eq!(chain.stats.symbols_extracted, 2); + } +} diff --git a/src/engine/context/resolver.rs b/src/engine/context/resolver.rs new file mode 100644 index 0000000..46521a3 --- /dev/null +++ b/src/engine/context/resolver.rs @@ -0,0 +1,921 @@ +//! Symbol resolution — map extracted symbols to file locations, +//! read source content, and assemble the context chain under a token budget. +//! +//! Resolution strategy (language-aware): +//! - **Rust**: `use crate::foo::bar` → `src/foo/bar.rs` (or `mod.rs`) +//! - **Python**: `import foo.bar` → `foo/bar.py` or `foo/bar/__init__.py` +//! - **JS/TS**: `import x from './foo'` → relative path resolution +//! - **Go**: `import "pkg/path"` → `pkg/path/` directory +//! - **Java/Kotlin**: `import foo.Bar` → `foo/Bar.java` / `foo/Bar.kt` +//! +//! Additionally, test file mapping is supported via naming conventions. + +use std::collections::HashMap; +use std::path::Path; + +use tracing::debug; + +use super::types::{ + ContextChain, ContextConfig, ContextEntry, ContextPriority, ContextStats, ExtractedSymbol, + SymbolKind, estimate_tokens, +}; + +/// Maximum lines to read per symbol definition (prevents reading huge functions). +const MAX_FN_LINES: usize = 50; +/// Maximum lines to read per struct/type definition. +const MAX_TYPE_LINES: usize = 50; +/// Maximum lines to read per test function. +const MAX_TEST_LINES: usize = 30; + +/// Build the full context chain from extracted symbols. +/// +/// This is the main entry point: extract → resolve → read → budget → assemble. +pub fn build_context_chain( + symbols: &[ExtractedSymbol], + config: &ContextConfig, + project_root: &Path, + ignore_patterns: &[String], +) -> ContextChain { + if !config.enabled || symbols.is_empty() { + return ContextChain::default(); + } + + let mut stats = ContextStats { + symbols_extracted: symbols.len(), + ..Default::default() + }; + + // Phase 1: Resolve symbols to file locations + let mut entries = resolve_symbols(symbols, config, project_root, ignore_patterns, &mut stats); + + // Phase 2: Add test file mappings + if config.include_tests { + add_test_mappings(symbols, project_root, &mut entries, &mut stats); + } + + // Sort by priority (FunctionDef first, Test last) + entries.sort_by_key(|e| e.priority); + + // Phase 3: Read file content under budget + let mut budget = config.max_context_tokens; + let mut parts = Vec::new(); + + for entry in &entries { + let content = read_entry_content(entry, project_root); + if content.is_empty() { + continue; + } + + let tokens = estimate_tokens(&content); + if tokens > budget { + stats.budget_hit = true; + debug!( + entry = %entry.label, + tokens, + remaining_budget = budget, + "skipping context entry (budget exhausted)" + ); + continue; + } + + budget -= tokens; + stats.entries_read += 1; + stats.estimated_tokens += tokens; + + parts.push(format!( + "--- {}:{}-{} ({}) ---\n{}", + entry.file, entry.line_start, entry.line_end, entry.label, content + )); + } + + let text = if parts.is_empty() { + String::new() + } else { + format!("Relevant Cross-File Context:\n\n{}", parts.join("\n")) + }; + + debug!( + symbols_extracted = stats.symbols_extracted, + symbols_resolved = stats.symbols_resolved, + entries_read = stats.entries_read, + estimated_tokens = stats.estimated_tokens, + budget_hit = stats.budget_hit, + "context chain built" + ); + + ContextChain { text, stats } +} + +/// Resolve extracted symbols to concrete file locations and line ranges. +fn resolve_symbols( + symbols: &[ExtractedSymbol], + config: &ContextConfig, + project_root: &Path, + ignore_patterns: &[String], + stats: &mut ContextStats, +) -> Vec { + let mut entries = Vec::new(); + let mut seen_files: HashMap> = HashMap::new(); // track line ranges per file + + for sym in symbols { + // Determine the language from the file extension + let lang = Path::new(&sym.file) + .extension() + .and_then(|e| e.to_str()) + .unwrap_or(""); + + 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), + }; + + for entry in resolved { + // Check ignore patterns + if is_ignored(&entry.file, ignore_patterns) { + debug!(file = %entry.file, "skipping ignored file"); + continue; + } + + // Check if the entry's file exists + let full_path = project_root.join(&entry.file); + if !full_path.exists() { + debug!(file = %entry.file, "resolved file does not exist, skipping"); + continue; + } + + // Don't add context for the same file the symbol came from + if entry.file == sym.file { + continue; + } + + stats.symbols_resolved += 1; + + // Merge line ranges for same file to avoid duplicates + let file_ranges = seen_files.entry(entry.file.clone()).or_default(); + let overlaps = file_ranges + .iter() + .any(|(s, e)| entry.line_start <= *e && entry.line_end >= *s); + + if !overlaps { + file_ranges.push((entry.line_start, entry.line_end)); + entries.push(entry); + } + } + + // Respect follow depth (depth 1 = only direct references) + if config.follow_depth <= 1 { + continue; + } + // Higher depths would recursively resolve symbols found in resolved content. + // For now, depth > 1 is a no-op placeholder for future expansion. + } + + entries +} + +/// Resolve an import to a file path. +fn resolve_import( + import_path: &str, + _source_file: &str, + lang: &str, + project_root: &Path, +) -> Vec { + let mut entries = Vec::new(); + + match lang { + "rs" => { + // `use crate::foo::bar::baz` → try `src/foo/bar/baz.rs` or `src/foo/bar/baz/mod.rs` + let path = import_path.replace("::", "/"); + let candidates = [ + format!("src/{path}.rs"), + format!("src/{path}/mod.rs"), + format!("src/{path}/lib.rs"), + ]; + + for candidate in &candidates { + let full = project_root.join(candidate); + if full.exists() { + let line_end = find_definition_end(&full); + entries.push(ContextEntry { + file: candidate.clone(), + line_start: 1, + line_end, + label: format!("module {import_path}"), + priority: ContextPriority::TypeDef, + }); + break; + } + } + } + "py" | "pyi" => { + // `import foo.bar` → `foo/bar.py` or `foo/bar/__init__.py` + let candidates = [ + format!("{import_path}.py"), + format!("{import_path}/__init__.py"), + ]; + + for candidate in &candidates { + let full = project_root.join(candidate); + if full.exists() { + let line_end = find_definition_end(&full); + entries.push(ContextEntry { + file: candidate.clone(), + line_start: 1, + line_end, + label: format!("module {import_path}"), + priority: ContextPriority::TypeDef, + }); + break; + } + } + } + "ts" | "tsx" | "js" | "jsx" | "mjs" | "cjs" => { + // `import x from './foo'` or `import x from 'foo'` → relative or node_modules + let path = import_path + .trim_start_matches("./") + .trim_start_matches("../"); + + // Only resolve relative imports (not node_modules) + if import_path.starts_with('.') { + let source_dir = Path::new(_source_file).parent().unwrap_or(Path::new("")); + let resolved = source_dir.join(path); + + let extensions = match lang { + "ts" | "tsx" => vec!["ts", "tsx"], + _ => vec!["js", "jsx", "mjs", "cjs"], + }; + + for ext in &extensions { + let candidate = format!("{}.{}", resolved.display(), ext); + let full = project_root.join(&candidate); + if full.exists() { + let line_end = find_definition_end(&full); + entries.push(ContextEntry { + file: candidate, + line_start: 1, + line_end, + label: format!("module {import_path}"), + priority: ContextPriority::TypeDef, + }); + break; + } + } + } + } + "go" => { + // Go imports are package paths; resolve relative to project root + let candidate = import_path.trim_start_matches("\"").trim_end_matches("\""); + let full = project_root.join(candidate); + if full.is_dir() { + // Find .go files in the directory + if let Some(entry) = find_go_package_file(&full, import_path) { + entries.push(entry); + } + } + } + "java" | "kt" | "kts" => { + // `import foo.bar.Baz` → `foo/bar/Baz.java` or `foo/bar/Baz.kt` + let path = import_path.replace('.', "/"); + let ext = if lang == "java" { "java" } else { "kt" }; + let candidate = format!("{path}.{ext}"); + let full = project_root.join(&candidate); + if full.exists() { + let line_end = find_definition_end(&full); + entries.push(ContextEntry { + file: candidate, + line_start: 1, + line_end, + label: format!("module {import_path}"), + priority: ContextPriority::TypeDef, + }); + } + } + _ => {} + } + + entries +} + +/// Resolve a function name to a file location. +/// This does a best-effort search for the function definition. +fn resolve_function( + name: &str, + source_file: &str, + lang: &str, + project_root: &Path, +) -> Vec { + let mut entries = Vec::new(); + + // Strategy: look in nearby files (same directory) and import targets + let search_dir = Path::new(source_file).parent().unwrap_or(Path::new("")); + + match lang { + "rs" => { + // Search for `pub fn ` or `fn ` in sibling .rs files + if let Ok(files) = std::fs::read_dir(project_root.join(search_dir)) { + for file in files.flatten() { + let path = file.path(); + if let Some(ext) = path.extension() { + if ext != "rs" { + continue; + } + } else { + continue; + } + + if let Ok(rel) = path.strip_prefix(project_root) { + let rel_str = rel.to_string_lossy().to_string(); + if rel_str == source_file { + continue; + } + + if let Some((start, end)) = find_fn_in_file(&path, name, "rs") { + entries.push(ContextEntry { + file: rel_str, + line_start: start, + line_end: end, + label: format!("fn {name}"), + priority: ContextPriority::FunctionDef, + }); + } + } + } + } + } + "py" | "pyi" => { + if let Ok(files) = std::fs::read_dir(project_root.join(search_dir)) { + for file in files.flatten() { + let path = file.path(); + if path.extension().map(|e| e != "py").unwrap_or(true) { + continue; + } + + if let Ok(rel) = path.strip_prefix(project_root) { + let rel_str = rel.to_string_lossy().to_string(); + if rel_str == source_file { + continue; + } + + if let Some((start, end)) = find_fn_in_file(&path, name, "py") { + entries.push(ContextEntry { + file: rel_str, + line_start: start, + line_end: end, + label: format!("def {name}"), + priority: ContextPriority::FunctionDef, + }); + } + } + } + } + } + _ => { + // Generic: search for the function name in sibling files + if let Ok(files) = std::fs::read_dir(project_root.join(search_dir)) { + for file in files.flatten() { + let path = file.path(); + if let Ok(rel) = path.strip_prefix(project_root) { + let rel_str = rel.to_string_lossy().to_string(); + if rel_str == source_file { + continue; + } + + if let Some((start, end)) = find_fn_generic(&path, name) { + entries.push(ContextEntry { + file: rel_str, + line_start: start, + line_end: end, + label: format!("fn {name}"), + priority: ContextPriority::FunctionDef, + }); + } + } + } + } + } + } + + entries +} + +/// Resolve a type name to a file location. +fn resolve_type( + name: &str, + source_file: &str, + lang: &str, + project_root: &Path, +) -> Vec { + let search_dir = Path::new(source_file).parent().unwrap_or(Path::new("")); + let mut entries = Vec::new(); + + let pattern = match lang { + "rs" => format!("struct {name}"), + "py" => format!("class {name}"), + "go" => format!("type {name} struct"), + "java" | "kt" => format!("class {name}"), + _ => format!("struct {name}"), + }; + + if let Ok(files) = std::fs::read_dir(project_root.join(search_dir)) { + for file in files.flatten() { + let path = file.path(); + if let Ok(rel) = path.strip_prefix(project_root) { + let rel_str = rel.to_string_lossy().to_string(); + if rel_str == source_file { + continue; + } + + if let Some((start, end)) = find_pattern_in_file(&path, &pattern) { + entries.push(ContextEntry { + file: rel_str, + line_start: start, + line_end: end, + label: format!("type {name}"), + priority: ContextPriority::TypeDef, + }); + } + } + } + } + + 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()?; + + 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, + }; + + find_pattern_with_body(&content, &pattern, max_lines) +} + +/// Generic function search (for languages without specific patterns). +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) +} + +/// Find a pattern (like `struct Foo`) and determine its extent. +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) +} + +/// 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; + let mut brace_count = 0i32; + let mut line_idx = 0; + + for line in content.lines() { + line_idx += 1; + + if start_line.is_none() { + if line.contains(pattern) + && !line.trim_start().starts_with("//") + && !line.trim_start().starts_with('#') + { + start_line = Some(line_idx); + brace_count = count_braces_delta(line); + } + continue; + } + + brace_count += count_braces_delta(line); + + // Block ends when braces are balanced (and we have at least the header) + if brace_count <= 0 && start_line.is_some() { + let start = start_line? as u32; + let end = (line_idx as u32).min(start + max_lines as u32); + return Some((start, end)); + } + + // Hard cap + if line_idx >= start_line.unwrap_or(0) + max_lines { + let start = start_line? as u32; + return Some((start, start + max_lines as u32)); + } + } + + // If we found the start but never balanced braces, cap at max_lines + start_line.map(|s| { + ( + s as u32, + (s + max_lines.min(content.lines().count() - s + 1)) as u32, + ) + }) +} + +/// Count net brace delta: +1 for `{`, -1 for `}`. +fn count_braces_delta(line: &str) -> i32 { + let mut delta = 0i32; + let mut in_string = false; + let mut in_char = false; + let mut escape = false; + + for ch in line.chars() { + if escape { + escape = false; + continue; + } + if ch == '\\' && !in_char { + escape = true; + continue; + } + if ch == '"' && !in_char { + in_string = !in_string; + continue; + } + if ch == '\'' && !in_string && !in_char { + in_char = true; + continue; + } + if ch == '\'' && in_char { + in_char = false; + continue; + } + if in_string || in_char { + continue; + } + if ch == '{' { + delta += 1; + } else if ch == '}' { + delta -= 1; + } + } + delta +} + +/// Find the last line of a file's "definition block" (simplified). +fn find_definition_end(path: &Path) -> u32 { + if let Ok(content) = std::fs::read_to_string(path) { + let lines = content.lines().count(); + (lines as u32).min(MAX_TYPE_LINES as u32) + } else { + 1 + } +} + +/// Find a .go file in a package directory. +fn find_go_package_file(dir: &Path, import_path: &str) -> Option { + let files = std::fs::read_dir(dir).ok()?; + for file in files.flatten() { + let path = file.path(); + if path.extension().map(|e| e == "go").unwrap_or(false) { + let line_end = find_definition_end(&path); + let rel = path + .to_str() + .and_then(|p| p.rsplit_once('/').map(|x| x.0)) + .unwrap_or(import_path); + return Some(ContextEntry { + file: rel.to_string(), + line_start: 1, + line_end, + label: format!("package {import_path}"), + priority: ContextPriority::TypeDef, + }); + } + } + None +} + +/// Check if a file path matches any ignore pattern. +fn is_ignored(file: &str, patterns: &[String]) -> bool { + for pattern in patterns { + // Simple glob-like matching: check if file contains the pattern + // or matches as a suffix (e.g., "target/**" matches "target/debug/foo.rs") + let p = pattern.trim_end_matches("**"); + if p.is_empty() { + continue; + } + if file.starts_with(p.trim_end_matches('/')) || file.contains(p.trim_matches('*')) { + return true; + } + } + false +} + +/// Add test file mappings for changed source files. +fn add_test_mappings( + symbols: &[ExtractedSymbol], + project_root: &Path, + entries: &mut Vec, + stats: &mut ContextStats, +) { + // Collect unique source files from symbols + let mut seen = std::collections::HashSet::new(); + + for sym in symbols { + if !seen.insert(&sym.file) { + continue; + } + + let candidates = test_file_candidates(&sym.file); + for candidate in candidates { + let full = project_root.join(&candidate); + if full.exists() { + let line_end = find_definition_end(&full); + entries.push(ContextEntry { + file: candidate, + line_start: 1, + line_end, + label: format!("tests for {}", sym.file), + priority: ContextPriority::Test, + }); + stats.symbols_resolved += 1; + break; + } + } + } +} + +/// Generate test file candidate paths for a source file. +fn test_file_candidates(source: &str) -> Vec { + let mut candidates = Vec::new(); + + let stem = Path::new(source) + .file_stem() + .and_then(|s| s.to_str()) + .unwrap_or(""); + + // Rust: `src/foo/bar.rs` → `tests/foo/bar_test.rs`, `tests/bar_test.rs` + // Also check: `tests/foo_test.rs` + if source.ends_with(".rs") { + candidates.push(format!("tests/{stem}_test.rs")); + if let Some(parent) = Path::new(source).parent() { + candidates.push(format!("tests/{}/{}", parent.display(), stem)); + candidates.push(format!("tests/{}/{}_test.rs", parent.display(), stem)); + } + candidates.push(format!("{stem}_test.rs")); + } + + // Python: `foo/bar.py` → `tests/test_bar.py`, `foo/test_bar.py` + if source.ends_with(".py") { + candidates.push(format!("tests/test_{stem}.py")); + if let Some(parent) = Path::new(source).parent() { + candidates.push(format!("{}/test_{stem}.py", parent.display())); + } + } + + // JS/TS: `src/foo.ts` → `src/foo.test.ts`, `tests/foo.test.ts` + if source.ends_with(".ts") || source.ends_with(".tsx") { + let ext = source.rsplit('.').next().unwrap_or("ts"); + candidates.push(format!("tests/{stem}.test.{ext}")); + candidates.push(format!("tests/{stem}.spec.{ext}")); + let without_ext = &source[..source.len() - ext.len() - 1]; + candidates.push(format!("{without_ext}.test.{ext}")); + candidates.push(format!("{without_ext}.spec.{ext}")); + } + + // Go: `foo.go` → `foo_test.go` + if source.ends_with(".go") { + candidates.push(source.replace(".go", "_test.go")); + } + + // Java/Kotlin + if source.ends_with(".java") { + candidates.push(source.replace(".java", "Test.java")); + } + if source.ends_with(".kt") { + candidates.push(source.replace(".kt", "Test.kt")); + } + + candidates +} + +/// Read the content for a context entry, respecting line range and caps. +fn read_entry_content(entry: &ContextEntry, project_root: &Path) -> String { + let full_path = project_root.join(&entry.file); + let content = match std::fs::read_to_string(&full_path) { + Ok(c) => c, + Err(e) => { + debug!(file = %entry.file, error = %e, "failed to read context entry file"); + return String::new(); + } + }; + + let start = entry.line_start.saturating_sub(1) as usize; + let end = entry.line_end as usize; + + let lines: Vec<&str> = content.lines().collect(); + let relevant: Vec<&str> = lines + .into_iter() + .skip(start) + .take(end.saturating_sub(start)) + .collect(); + + let result = relevant.join("\n"); + + // Apply cap based on priority + let max_lines = match entry.priority { + ContextPriority::FunctionDef => MAX_FN_LINES, + ContextPriority::TypeDef => MAX_TYPE_LINES, + ContextPriority::Test => MAX_TEST_LINES, + }; + + if result.lines().count() > max_lines { + result + .lines() + .take(max_lines) + .chain(std::iter::once("... (truncated)")) + .collect::>() + .join("\n") + } else { + result + } +} + +#[cfg(test)] +mod tests { + use super::*; + + // ─── test_file_candidates ─── + + #[test] + fn rust_test_candidates() { + let candidates = test_file_candidates("src/engine/scanner.rs"); + assert!( + candidates.contains(&"tests/scanner_test.rs".to_string()), + "should generate tests/scanner_test.rs" + ); + assert!( + candidates.iter().any(|c| c.ends_with("engine/scanner")), + "should generate nested test path" + ); + } + + #[test] + fn python_test_candidates() { + let candidates = test_file_candidates("app/auth.py"); + assert!( + candidates.contains(&"tests/test_auth.py".to_string()), + "should generate tests/test_auth.py" + ); + } + + #[test] + fn js_test_candidates() { + let candidates = test_file_candidates("src/api.ts"); + assert!( + candidates.iter().any(|c| c.contains("api.test.ts")), + "should generate .test.ts candidate" + ); + } + + #[test] + fn go_test_candidates() { + let candidates = test_file_candidates("main.go"); + assert!( + candidates.contains(&"main_test.go".to_string()), + "should generate main_test.go" + ); + } + + // ─── is_ignored ─── + + #[test] + fn ignore_target_dir() { + assert!(is_ignored( + "target/debug/foo.rs", + &["target/**".to_string()] + )); + } + + #[test] + fn ignore_node_modules() { + assert!(is_ignored( + "node_modules/pkg/index.js", + &["node_modules/**".to_string()] + )); + } + + #[test] + fn not_ignored_src() { + assert!(!is_ignored("src/main.rs", &["target/**".to_string()])); + } + + // ─── count_braces_delta ─── + + #[test] + fn brace_delta_basic() { + assert_eq!(count_braces_delta("{ }"), 0); + assert_eq!(count_braces_delta("{{"), 2); + assert_eq!(count_braces_delta("}}"), -2); + assert_eq!(count_braces_delta("fn foo() {"), 1); + assert_eq!(count_braces_delta("}"), -1); + } + + #[test] + fn brace_delta_ignores_strings() { + assert_eq!(count_braces_delta(r#"let s = "{";"#), 0); + assert_eq!(count_braces_delta(r#"println!("{")");"#), 0); + } + + #[test] + fn brace_delta_ignores_comments() { + // Brace in comment shouldn't count... but our simple parser doesn't handle + // Rust comments. For now, it's a known limitation. The function handles + // string escaping correctly though. + assert_eq!(count_braces_delta(r#"let x = 1; // { }"#), 0); + } + + // ─── find_pattern_with_body ─── + + #[test] + fn find_simple_struct() { + let content = "fn main() {}\n\npub struct Foo {\n x: i32,\n}\n\nfn other() {}"; + let result = find_pattern_with_body(content, "struct Foo", 10); + assert_eq!(result, Some((3, 5))); + } + + #[test] + fn find_nested_function() { + let content = "fn outer() {\n fn inner() {\n 1\n }\n}\n\npub fn target() {\n let x = 1;\n return x;\n}\n"; + let result = find_pattern_with_body(content, "fn target", 10); + assert_eq!(result, Some((7, 10))); + } + + #[test] + fn find_pattern_not_found() { + let content = "fn main() {}\npub fn other() {}"; + let result = find_pattern_with_body(content, "fn missing", 10); + assert!(result.is_none()); + } + + // ─── build_context_chain integration ─── + + #[test] + fn disabled_config_returns_empty() { + let config = ContextConfig { + enabled: false, + ..Default::default() + }; + let chain = build_context_chain(&[], &config, Path::new("/tmp"), &[]); + assert!(chain.text.is_empty()); + } + + #[test] + fn empty_symbols_returns_empty() { + let config = ContextConfig::default(); + let chain = build_context_chain(&[], &config, Path::new("/tmp"), &[]); + assert!(chain.text.is_empty()); + } + + #[test] + fn budget_enforced() { + // Create a tiny budget + let config = ContextConfig { + enabled: true, + max_context_tokens: 5, // very small + follow_depth: 1, + include_tests: false, + }; + + let symbols = vec![ExtractedSymbol { + kind: SymbolKind::FunctionCall("some_func".to_string()), + file: "nonexistent.rs".to_string(), + line: 1, + raw: "some_func()".to_string(), + }]; + + let chain = build_context_chain(&symbols, &config, Path::new("/tmp"), &[]); + // Even if resolution finds nothing, the chain should be empty + assert!(chain.text.is_empty() || chain.stats.budget_hit); + } + + // ─── estimate_tokens consistency ─── + + #[test] + fn budget_accounting() { + let config = ContextConfig { + enabled: true, + max_context_tokens: 100, + follow_depth: 1, + include_tests: false, + }; + + let symbols = vec![ExtractedSymbol { + kind: SymbolKind::Import("engine::scanner".to_string()), + file: "src/main.rs".to_string(), + line: 1, + raw: "use crate::engine::scanner;".to_string(), + }]; + + let chain = build_context_chain(&symbols, &config, Path::new("/tmp"), &[]); + // With a nonexistent project root, nothing should resolve + assert!(chain.text.is_empty()); + assert_eq!(chain.stats.symbols_extracted, 1); + } +} diff --git a/src/engine/context/types.rs b/src/engine/context/types.rs new file mode 100644 index 0000000..4822c56 --- /dev/null +++ b/src/engine/context/types.rs @@ -0,0 +1,228 @@ +//! Context chain types — symbols, resolved references, and configuration. +//! +//! The context chain enriches LLM review prompts with cross-file dependency +//! information extracted deterministically from changed code, zero LLM tokens. + +use serde::{Deserialize, Serialize}; + +/// Configuration for the context chain feature. +/// +/// Lives inside `Config` and is also deserializable from `.cora.yaml` +/// under the `review.context_chain` key. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ContextConfig { + /// Whether the context chain is enabled. + /// When `false`, no cross-file context is collected. + #[serde(default = "default_true")] + pub enabled: bool, + + /// Maximum number of *tokens* of additional context to inject. + /// The budget is enforced via a rough `chars / 4` estimation. + /// Default: 3000 tokens ≈ 12 KB of code. + #[serde(default = "default_max_context_tokens")] + pub max_context_tokens: usize, + + /// Maximum levels of dependency following. + /// Depth 1 = resolve symbols from changed lines. + /// Depth 2 = also resolve symbols referenced in depth-1 results. + /// Depth 3+ follows further. Default: 1 (only direct references). + #[serde(default = "default_follow_depth")] + pub follow_depth: u32, + + /// Whether to auto-resolve test files via naming convention. + /// For Rust: `src/foo.rs` → `tests/foo_test.rs` or `tests/foo_test/`. + /// Default: true. + #[serde(default = "default_true")] + pub include_tests: bool, +} + +fn default_true() -> bool { + true +} + +fn default_max_context_tokens() -> usize { + 3000 +} + +fn default_follow_depth() -> u32 { + 1 +} + +impl Default for ContextConfig { + fn default() -> Self { + Self { + enabled: true, + max_context_tokens: 3000, + follow_depth: 1, + include_tests: true, + } + } +} + +/// A symbol extracted from source code in changed lines. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SymbolKind { + /// Function or method call (e.g., `validate_token()`, `self.handle()`). + FunctionCall(String), + /// Type or struct reference (e.g., `CryptoConfig`, `HashMap`). + TypeRef(String), + /// Import/use statement pointing to a module path + /// (e.g., `use crate::engine::scanner`). + Import(String), +} + +impl std::fmt::Display for SymbolKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + SymbolKind::FunctionCall(name) => write!(f, "fn {name}"), + SymbolKind::TypeRef(name) => write!(f, "type {name}"), + SymbolKind::Import(path) => write!(f, "import {path}"), + } + } +} + +/// A single symbol extracted from a changed line, with source location. +#[derive(Debug, Clone)] +#[allow(dead_code)] // fields consumed in v0.5 when bundling wired +pub struct ExtractedSymbol { + /// What kind of symbol this is. + pub kind: SymbolKind, + /// The file path where the symbol was found. + pub file: String, + /// Line number in the file (1-indexed). + pub line: u32, + /// The raw text that matched (for debugging). + pub raw: String, +} + +/// A resolved context entry — a (file, line range, symbol) tuple ready +/// for reading and injection into the prompt. +#[derive(Debug, Clone)] +pub struct ContextEntry { + /// Resolved file path (relative to project root). + pub file: String, + /// Start line (1-indexed, inclusive). + pub line_start: u32, + /// End line (1-indexed, inclusive). + pub line_end: u32, + /// Human-readable label (e.g., `fn validate_token`, `struct CryptoConfig`). + pub label: String, + /// Priority tier for budget allocation. + pub priority: ContextPriority, +} + +/// Priority of a context entry — controls which entries are included +/// first when the token budget is limited. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub enum ContextPriority { + /// Function/method definitions — highest priority. + FunctionDef = 0, + /// Type/struct definitions. + TypeDef = 1, + /// Test functions — lowest priority. + Test = 2, +} + +/// Statistics from a context chain build, useful for logging / progress events. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct ContextStats { + /// Number of symbols extracted from changed lines. + pub symbols_extracted: usize, + /// Number of symbols successfully resolved to file locations. + pub symbols_resolved: usize, + /// Number of context entries read from disk. + pub entries_read: usize, + /// Estimated token count of the assembled context. + pub estimated_tokens: usize, + /// Whether the budget was hit (some entries were dropped). + pub budget_hit: bool, +} + +/// The fully assembled context chain, ready for prompt injection. +#[derive(Debug, Clone, Default)] +#[allow(dead_code)] // stats consumed in v0.5 when bundling wired +pub struct ContextChain { + /// Formatted text to inject into the prompt. + pub text: String, + /// Build statistics. + pub stats: ContextStats, +} + +/// Rough token estimation: ~4 characters per token. +/// This is a heuristic — consistent across runs, which matters more than accuracy. +pub fn estimate_tokens(text: &str) -> usize { + text.len() / 4 +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn context_config_default() { + let cfg = ContextConfig::default(); + assert!(cfg.enabled); + assert_eq!(cfg.max_context_tokens, 3000); + assert_eq!(cfg.follow_depth, 1); + assert!(cfg.include_tests); + } + + #[test] + fn context_config_deserialize_partial() { + let yaml = "enabled: false\nmax_context_tokens: 5000\n"; + let cfg: ContextConfig = serde_yaml_ng::from_str(yaml).unwrap(); + assert!(!cfg.enabled); + assert_eq!(cfg.max_context_tokens, 5000); + // Defaults should fill in for missing fields + assert_eq!(cfg.follow_depth, 1); + assert!(cfg.include_tests); + } + + #[test] + fn context_config_deserialize_full() { + let yaml = + "enabled: true\nmax_context_tokens: 2000\nfollow_depth: 3\ninclude_tests: false\n"; + let cfg: ContextConfig = serde_yaml_ng::from_str(yaml).unwrap(); + assert!(cfg.enabled); + assert_eq!(cfg.max_context_tokens, 2000); + assert_eq!(cfg.follow_depth, 3); + assert!(!cfg.include_tests); + } + + #[test] + fn estimate_tokens_basic() { + // 40 chars → ~10 tokens + assert_eq!( + estimate_tokens("hello world this is a test string xyz123"), + 10 + ); + } + + #[test] + fn estimate_tokens_empty() { + assert_eq!(estimate_tokens(""), 0); + } + + #[test] + fn estimate_tokens_short() { + // 3 chars → 0 tokens (integer division) + assert_eq!(estimate_tokens("abc"), 0); + } + + #[test] + fn symbol_kind_display() { + let fc = format!("{}", SymbolKind::FunctionCall("foo".into())); + assert_eq!(fc, "fn foo"); + let tr = format!("{}", SymbolKind::TypeRef("Bar".into())); + assert_eq!(tr, "type Bar"); + let im = format!("{}", SymbolKind::Import("crate::mod".into())); + assert_eq!(im, "import crate::mod"); + } + + #[test] + fn context_priority_ordering() { + // FunctionDef < TypeDef < Test (Ord — smaller = higher priority) + assert!(ContextPriority::FunctionDef < ContextPriority::TypeDef); + assert!(ContextPriority::TypeDef < ContextPriority::Test); + } +} diff --git a/src/engine/diff_parser.rs b/src/engine/diff_parser.rs new file mode 100644 index 0000000..28eaec0 --- /dev/null +++ b/src/engine/diff_parser.rs @@ -0,0 +1,569 @@ +/// Unified diff parser — converts raw diff text into structured [`FileChunk`] data. +/// +/// Simple regex-based parser that handles: +/// - Added, modified, deleted, and binary files +/// - Renames (old → new path) +/// - Multi-hunk diffs with line tracking +/// - Language detection from file extensions +use regex::Regex; +use std::sync::LazyLock; +use tracing::debug; + +/// A single file's worth of changes in a diff. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FileChunk { + /// Old file path (from `--- a/path`). `None` for new files. + pub old_path: Option, + /// New file path (from `+++ b/path`). `None` for deleted files. + pub new_path: Option, + /// Language detected from file extension (e.g., `"rs"`, `"py"`). + pub language: String, + /// The `@@ ... @@` hunks inside this file. + pub chunks: Vec, + /// Whether this is a binary file change. + pub is_binary: bool, + /// Whether this file was deleted. + pub is_deleted: bool, + /// Whether this file is newly created. + pub is_new: bool, +} + +/// A single `@@ ... @@` hunk block. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DiffHunk { + pub old_start: u32, + pub old_count: u32, + pub new_start: u32, + pub new_count: u32, + /// The header text after the `@@` markers (function context). + pub header: String, + /// Individual diff lines within this hunk. + pub lines: Vec, +} + +/// A single line inside a diff hunk. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DiffLine { + /// Whether this line was added, removed, or is context. + pub line_type: DiffLineType, + /// Content stripped of the leading `+`/`-`/` ` prefix. + pub content: String, + /// Line number in the old file (for `Remove` and `Context` lines). + pub old_line_no: Option, + /// Line number in the new file (for `Add` and `Context` lines). + pub new_line_no: Option, +} + +/// Type of a diff line. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum DiffLineType { + Add, + Remove, + Context, +} + +// ─── Regex patterns (compiled once) ─── + +static RE_FILE_HEADER_OLD: LazyLock = + LazyLock::new(|| Regex::new(r"^---\s+(?:a/)?(.*)").unwrap()); +static RE_FILE_HEADER_NEW: LazyLock = + LazyLock::new(|| Regex::new(r"^\+\+\+\s+(?:b/)?(.*)").unwrap()); +static RE_HUNK_HEADER: LazyLock = + LazyLock::new(|| Regex::new(r"^@@\s-(\d+)(?:,(\d+))?\s\+(\d+)(?:,(\d+))?\s@@\s?(.*)").unwrap()); +static RE_BINARY: LazyLock = LazyLock::new(|| Regex::new(r"^Binary files").unwrap()); + +/// Parse a unified diff string into a list of [`FileChunk`] entries. +pub fn parse_diff(diff: &str) -> Vec { + let mut files = Vec::new(); + let mut current: Option = None; + + for line in diff.lines() { + // Check for file header pair (--- / +++) + if let Some(caps) = RE_FILE_HEADER_OLD.captures(line) { + // If we have a pending file, flush it + if let Some(builder) = current.take() { + files.push(builder.build()); + } + let raw_path = caps[1].trim_end_matches('\t').to_string(); + let is_new = raw_path == "/dev/null"; + + let mut builder = FileChunkBuilder::new(); + builder.old_path = if is_new { None } else { Some(raw_path) }; + builder.is_new = is_new; + current = Some(builder); + continue; + } + + if let Some(ref mut builder) = current { + if let Some(caps) = RE_FILE_HEADER_NEW.captures(line) { + let raw_path = caps[1].trim_end_matches('\t').to_string(); + let is_deleted = raw_path == "/dev/null"; + + builder.new_path = if is_deleted { None } else { Some(raw_path) }; + builder.is_deleted = is_deleted; + // Determine language from the new path (preferred) or old path + let lang_path = builder.new_path.as_deref().or(builder.old_path.as_deref()); + if let Some(p) = lang_path { + builder.language = detect_language(p).to_string(); + } + continue; + } + } + + // Inside a file section, check for hunks and lines + if let Some(ref mut builder) = current { + // Check for binary marker (before hunk check — binary files have no hunks) + if RE_BINARY.is_match(line) { + builder.is_binary = true; + continue; + } + + if let Some(caps) = RE_HUNK_HEADER.captures(line) { + // Flush previous hunk (multi-hunk support) + if let Some(hunk) = builder.current_hunk.take() { + builder.hunks.push(hunk); + } + + let old_start: u32 = caps[1].parse().unwrap_or(1); + let old_count: u32 = caps[2].parse().unwrap_or(1); + let new_start: u32 = caps[3].parse().unwrap_or(1); + let new_count: u32 = caps[4].parse().unwrap_or(1); + let header = caps[5].to_string(); + + // Initialize line counters from hunk header + builder.old_line = old_start; + builder.new_line = new_start; + + builder.current_hunk = Some(DiffHunk { + old_start, + old_count, + new_start, + new_count, + header, + lines: Vec::new(), + }); + continue; + } + + if let Some(ref mut hunk) = builder.current_hunk { + // Parse diff lines + let (line_type, content) = if let Some(rest) = line.strip_prefix('+') { + (DiffLineType::Add, rest.to_string()) + } else if let Some(rest) = line.strip_prefix('-') { + (DiffLineType::Remove, rest.to_string()) + } else if let Some(rest) = line.strip_prefix(' ') { + (DiffLineType::Context, rest.to_string()) + } else { + // Unknown line (e.g., "diff --git ..."), skip + continue; + }; + + // Compute line numbers based on line type, then increment counters + let (old_line_no, new_line_no) = match line_type { + DiffLineType::Add => { + let no = if builder.new_line > 0 { + Some(builder.new_line) + } else { + None + }; + builder.new_line += 1; + (None, no) + } + DiffLineType::Remove => { + let no = if builder.old_line > 0 { + Some(builder.old_line) + } else { + None + }; + builder.old_line += 1; + (no, None) + } + DiffLineType::Context => { + let old_no = if builder.old_line > 0 { + Some(builder.old_line) + } else { + None + }; + let new_no = if builder.new_line > 0 { + Some(builder.new_line) + } else { + None + }; + builder.old_line += 1; + builder.new_line += 1; + (old_no, new_no) + } + }; + + hunk.lines.push(DiffLine { + line_type, + content, + old_line_no, + new_line_no, + }); + } + } + } + + // Flush the last file + if let Some(builder) = current.take() { + files.push(builder.build()); + } + + debug!(file_count = files.len(), "parsed diff into file chunks"); + files +} + +/// Detect programming language from a file path's extension. +/// +/// Returns the language shorthand (e.g., `"rs"`, `"py"`, `"ts"`). +/// Falls back to `"unknown"` for unrecognized extensions. +pub fn detect_language(path: &str) -> &'static str { + match path.rsplit('.').next() { + Some(ext) => match ext.to_lowercase().as_str() { + "rs" => "rs", + "py" | "pyi" => "py", + "ts" | "tsx" => "ts", + "js" | "jsx" | "mjs" | "cjs" => "js", + "go" => "go", + "java" => "java", + "kt" | "kts" => "kt", + "c" => "c", + "cpp" | "cc" | "cxx" => "cpp", + "h" | "hpp" | "hxx" => "h", + "rb" => "rb", + "php" => "php", + "swift" => "swift", + "scala" => "scala", + "r" => "r", + "sql" => "sql", + "sh" | "bash" | "zsh" => "sh", + "toml" => "toml", + "yaml" | "yml" => "yaml", + "json" => "json", + "md" | "markdown" => "md", + "html" | "htm" => "html", + "css" | "scss" | "sass" | "less" => "css", + "dart" => "dart", + "lua" => "lua", + "zig" => "zig", + "ex" | "exs" => "ex", + "proto" => "proto", + "graphql" | "gql" => "graphql", + "dockerfile" => "dockerfile", + // Non-code extensions (still recognized for exclusion/filtering) + "png" | "jpg" | "jpeg" | "gif" | "svg" | "ico" | "webp" => "image", + "pdf" | "doc" | "docx" => "document", + "lock" | "sum" => "lock", + _ => "unknown", + }, + None => "unknown", + } +} + +/// Extract all added lines from parsed file chunks. +/// +/// Returns `(file_path, line_number, content)` triples. +#[allow(dead_code)] // used by bundling in v0.5 +pub fn extract_added_lines(chunks: &[FileChunk]) -> Vec<(String, u32, String)> { + let mut result = Vec::new(); + + for file in chunks { + let path = file + .new_path + .as_deref() + .or(file.old_path.as_deref()) + .unwrap_or("unknown"); + for hunk in &file.chunks { + for line in &hunk.lines { + if line.line_type == DiffLineType::Add { + if let Some(ln) = line.new_line_no { + result.push((path.to_string(), ln, line.content.clone())); + } + } + } + } + } + + result +} + +// ─── Internal builder for assembling FileChunk ─── + +struct FileChunkBuilder { + old_path: Option, + new_path: Option, + language: String, + current_hunk: Option, + hunks: Vec, + old_line: u32, + new_line: u32, + is_binary: bool, + is_deleted: bool, + is_new: bool, +} + +impl FileChunkBuilder { + fn new() -> Self { + Self { + old_path: None, + new_path: None, + language: "unknown".to_string(), + current_hunk: None, + hunks: Vec::new(), + old_line: 0, + new_line: 0, + is_binary: false, + is_deleted: false, + is_new: false, + } + } + + fn build(mut self) -> FileChunk { + if let Some(hunk) = self.current_hunk.take() { + self.hunks.push(hunk); + } + FileChunk { + old_path: self.old_path, + new_path: self.new_path, + language: self.language, + chunks: self.hunks, + is_binary: self.is_binary, + is_deleted: self.is_deleted, + is_new: self.is_new, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_simple_diff() { + let diff = r#"diff --git a/src/main.rs b/src/main.rs +--- a/src/main.rs ++++ b/src/main.rs +@@ -1,3 +1,4 @@ + use std::io; + fn main() { ++ println!("hello"); + } +"#; + let files = parse_diff(diff); + assert_eq!(files.len(), 1); + let f = &files[0]; + assert_eq!(f.old_path.as_deref(), Some("src/main.rs")); + assert_eq!(f.new_path.as_deref(), Some("src/main.rs")); + assert_eq!(f.language, "rs"); + assert_eq!(f.chunks.len(), 1); + assert_eq!(f.chunks[0].lines.len(), 4); + + let added: Vec<_> = f.chunks[0] + .lines + .iter() + .filter(|l| l.line_type == DiffLineType::Add) + .collect(); + assert_eq!(added.len(), 1); + assert!(added[0].content.contains("println")); + } + + #[test] + fn parse_new_file() { + let diff = r#"diff --git a/src/new_file.py b/src/new_file.py +new file mode 100644 +--- /dev/null ++++ b/src/new_file.py +@@ -0,0 +1,5 @@ ++import os ++ ++def main(): ++ pass ++ +"#; + let files = parse_diff(diff); + assert_eq!(files.len(), 1); + let f = &files[0]; + assert!(f.is_new); + assert!(!f.is_deleted); + assert!(f.old_path.is_none()); + assert_eq!(f.new_path.as_deref(), Some("src/new_file.py")); + assert_eq!(f.language, "py"); + } + + #[test] + fn parse_deleted_file() { + let diff = r#"diff --git a/old_file.go b/old_file.go +deleted file mode 100644 +--- a/old_file.go ++++ /dev/null +@@ -1,3 +0,0 @@ +-package main +- +-func deleted() {} +"#; + let files = parse_diff(diff); + assert_eq!(files.len(), 1); + let f = &files[0]; + assert!(f.is_deleted); + assert!(!f.is_new); + assert!(f.new_path.is_none()); + assert_eq!(f.old_path.as_deref(), Some("old_file.go")); + assert_eq!(f.language, "go"); + } + + #[test] + fn parse_binary_file() { + let diff = r#"diff --git a/image.png b/image.png +--- a/image.png ++++ b/image.png +Binary files a/image.png and b/image.png differ +"#; + let files = parse_diff(diff); + assert_eq!(files.len(), 1); + let f = &files[0]; + assert!(f.is_binary); + assert_eq!(f.language, "image"); + assert!(f.chunks.is_empty()); + } + + #[test] + fn parse_multifile_diff() { + let diff = r#"diff --git a/lib.rs b/lib.rs +--- a/lib.rs ++++ b/lib.rs +@@ -1,1 +1,2 @@ + pub mod foo; ++pub mod bar; +diff --git a/src/foo.rs b/src/foo.rs +--- a/src/foo.rs ++++ b/src/foo.rs +@@ -1,3 +1,3 @@ + fn foo() { +- old(); ++ new(); + } +"#; + let files = parse_diff(diff); + assert_eq!(files.len(), 2); + assert_eq!(files[0].new_path.as_deref(), Some("lib.rs")); + assert_eq!(files[1].new_path.as_deref(), Some("src/foo.rs")); + } + + #[test] + fn parse_multi_hunk_diff() { + let diff = r#"diff --git a/src/main.rs b/src/main.rs +--- a/src/main.rs ++++ b/src/main.rs +@@ -1,3 +1,3 @@ + fn a() { +- old_a(); ++ new_a(); + } +@@ -10,3 +10,3 @@ + fn b() { +- old_b(); ++ new_b(); + } +"#; + let files = parse_diff(diff); + assert_eq!(files.len(), 1); + assert_eq!(files[0].chunks.len(), 2); + assert_eq!(files[0].chunks[0].old_start, 1); + assert_eq!(files[0].chunks[1].old_start, 10); + } + + #[test] + fn line_numbers_tracked() { + let diff = r#"diff --git a/test.ts b/test.ts +--- a/test.ts ++++ b/test.ts +@@ -5,4 +5,5 @@ + const x = 1; +-const y = 2; ++const y = 3; ++const z = 4; + const w = 5; +"#; + let files = parse_diff(diff); + let hunk = &files[0].chunks[0]; + // Context line at old=5, new=5 + let ctx = &hunk.lines[0]; + assert_eq!(ctx.line_type, DiffLineType::Context); + assert_eq!(ctx.old_line_no, Some(5)); + assert_eq!(ctx.new_line_no, Some(5)); + // Removed line at old=6 + let rem = &hunk.lines[1]; + assert_eq!(rem.line_type, DiffLineType::Remove); + assert_eq!(rem.old_line_no, Some(6)); + assert!(rem.new_line_no.is_none()); + // First added line at new=6 + let add1 = &hunk.lines[2]; + assert_eq!(add1.line_type, DiffLineType::Add); + assert!(add1.old_line_no.is_none()); + assert_eq!(add1.new_line_no, Some(6)); + // Second added line at new=7 + let add2 = &hunk.lines[3]; + assert_eq!(add2.new_line_no, Some(7)); + } + + #[test] + fn extract_added_lines_works() { + let diff = r#"diff --git a/app.js b/app.js +--- a/app.js ++++ b/app.js +@@ -1,1 +1,3 @@ + console.log('a'); ++console.log('b'); ++console.log('c'); +"#; + let files = parse_diff(diff); + let added = extract_added_lines(&files); + assert_eq!(added.len(), 2); + assert_eq!(added[0].0, "app.js"); + assert_eq!(added[0].1, 2); + assert_eq!(added[1].1, 3); + } + + #[test] + fn detect_language_various() { + assert_eq!(detect_language("src/main.rs"), "rs"); + assert_eq!(detect_language("script.py"), "py"); + assert_eq!(detect_language("index.ts"), "ts"); + assert_eq!(detect_language("app.tsx"), "ts"); + assert_eq!(detect_language("server.js"), "js"); + assert_eq!(detect_language("main.go"), "go"); + assert_eq!(detect_language("App.java"), "java"); + assert_eq!(detect_language("Makefile"), "unknown"); + assert_eq!(detect_language("path/to/no_ext"), "unknown"); + assert_eq!(detect_language("style.scss"), "css"); + } + + #[test] + fn empty_diff_returns_empty() { + let files = parse_diff(""); + assert!(files.is_empty()); + } + + #[test] + fn hunk_header_with_function_context() { + let diff = r#"diff --git a/lib.rs b/lib.rs +--- a/lib.rs ++++ b/lib.rs +@@ -10,7 +10,8 @@ pub fn process(input: &str) { + let data = parse(input); +- consume(data); ++ let result = transform(data); ++ consume(result); + } +"#; + let files = parse_diff(diff); + assert_eq!(files.len(), 1); + let hunk = &files[0].chunks[0]; + assert_eq!(hunk.old_start, 10); + assert_eq!(hunk.old_count, 7); + assert_eq!(hunk.new_start, 10); + assert_eq!(hunk.new_count, 8); + assert!(hunk.header.contains("pub fn process")); + } +} diff --git a/src/engine/mod.rs b/src/engine/mod.rs index d4b01d0..d116145 100644 --- a/src/engine/mod.rs +++ b/src/engine/mod.rs @@ -1,6 +1,10 @@ +pub mod bundling; pub mod cache; +pub mod context; +pub mod diff_parser; pub mod llm; pub mod review; +pub mod rules; pub mod scanner; pub mod static_analysis; pub mod types; diff --git a/src/engine/review.rs b/src/engine/review.rs index 0f06421..152ac08 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -113,7 +113,43 @@ async fn review_diff_inner( let static_context = crate::engine::static_analysis::collect_static_context(diff, &config.static_analysis); - let mut response = if stream { + // Parse diff and run rule engine + let diff_chunks = crate::engine::diff_parser::parse_diff(diff); + let rule_findings = crate::engine::rules::run_rules(&diff_chunks, &config.rules_config); + let rule_context = crate::engine::rules::format_rule_context(&rule_findings); + // Keep a clone for merging after LLM (rule_findings may be consumed in error fallback) + let rule_findings_clone = rule_findings.clone(); + + // Combine static analysis + rule engine context for LLM prompt + let combined_context = match (static_context.as_deref(), rule_context.as_str()) { + (Some(sa), rc) if !rc.is_empty() => Some(format!("{sa}\n\n{rc}")), + (Some(sa), _) => Some(sa.to_string()), + (_, rc) if !rc.is_empty() => Some(rc.to_string()), + _ => None, + }; + + // Build context chain (cross-file dependency extraction) + let context_chain = crate::engine::context::build_context_chain( + &diff_chunks, + &config.context_chain, + std::env::current_dir().unwrap_or_default().as_path(), + &config.ignore.rules, + ); + + let final_context = if !context_chain.text.is_empty() { + match combined_context { + Some(ctx) => Some(format!( + "{ctx}\n\n## Cross-file Context\n{context_chain_text}", + context_chain_text = context_chain.text + )), + None => Some(format!("## Cross-file Context\n{}", context_chain.text)), + } + } else { + combined_context + }; + + // Call LLM — but preserve deterministic rule findings even on LLM failure + let llm_result: Result = if stream { llm::review_diff_stream( llm_config, diff, @@ -121,9 +157,9 @@ async fn review_diff_inner( &config.rules, &config.response_format, review_prompt.as_deref(), - static_context.as_deref(), + final_context.as_deref(), ) - .await? + .await } else { llm::review_diff( llm_config, @@ -133,11 +169,48 @@ async fn review_diff_inner( &config.response_format, review_prompt.as_deref(), quiet, - static_context.as_deref(), + final_context.as_deref(), ) - .await? + .await }; + let mut response = match llm_result { + Ok(resp) => resp, + Err(e) => { + // LLM failed — return deterministic findings only (don't silently swallow them) + if !rule_findings.is_empty() { + let n_rules = rule_findings.len(); + debug!( + error = %e, + rule_findings = n_rules, + "LLM call failed, returning deterministic rule findings only" + ); + let mut fallback = ReviewResponse { + issues: crate::engine::rules::merge_rule_findings(vec![], rule_findings), + summary: format!( + "LLM review failed: {e}. Showing {n_rules} deterministic rule findings only." + ), + tokens_used: None, + should_block: false, + }; + fallback.issues = apply_ignore_rules(fallback.issues, &config.ignore.rules); + let min_sev = config.hook.min_severity_level(); + fallback.should_block = fallback + .issues + .iter() + .any(|issue| issue.severity <= min_sev); + return Ok(fallback); + } + return Err(e); + } + }; + + // Merge rule findings with LLM issues (rule findings appended after LLM issues) + if !rule_findings_clone.is_empty() { + response.issues = + crate::engine::rules::merge_rule_findings(response.issues, rule_findings_clone); + } + // Filter out issues with invalid file paths (hallucination guard) if !valid_files.is_empty() { let before = response.issues.len(); diff --git a/src/engine/rules/builtin.rs b/src/engine/rules/builtin.rs new file mode 100644 index 0000000..12b2f32 --- /dev/null +++ b/src/engine/rules/builtin.rs @@ -0,0 +1,127 @@ +/// Built-in rules for the rule engine. +use crate::engine::Severity; +use crate::engine::rules::types::CustomRule; + +/// Returns the full list of built-in rules. +pub fn builtin_rules() -> Vec { + vec![ + // --- Security --- + CustomRule { + id: "sec-hardcoded-secret".to_string(), + pattern: r#"(?i)(?:password|api_?key|token|secret)\s*=\s*"[^"]+""# + .to_string(), + severity: Severity::Critical, + message: "Possible hardcoded secret/credential detected. Use environment variables or a secrets manager.".to_string(), + languages: vec!["all".to_string()], + exclude: vec![], + }, + CustomRule { + id: "sec-sql-concat".to_string(), + pattern: r##"format!\("SELECT|f"SELECT|f"INSERT|f"UPDATE|f"DELETE|query\s*\+="## + .to_string(), + severity: Severity::Critical, + message: "Possible SQL injection via string concatenation in query. Use parameterized queries.".to_string(), + languages: vec!["all".to_string()], + exclude: vec![], + }, + CustomRule { + id: "sec-hardcoded-url".to_string(), + pattern: r"http://[a-zA-Z0-9][\w.\-]+(:\d+)?(/\S*)?" + .to_string(), + severity: Severity::Major, + message: "Insecure HTTP URL detected (not https). Use HTTPS for all external connections.".to_string(), + languages: vec!["all".to_string()], + exclude: vec![], + }, + CustomRule { + id: "sec-tls-disabled".to_string(), + pattern: r"tls_built_in_root_certs\(false\)|verify\s*=\s*False|InsecureRequestWarning|ACCEPT_INVALID_CERTS|dangerAcceptAnyServerCert".to_string(), + severity: Severity::Critical, + 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![], + }, + // --- Bugs --- + CustomRule { + id: "bug-unwrap".to_string(), + pattern: r"\.unwrap\(\)".to_string(), + severity: Severity::Minor, + 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()], + }, + CustomRule { + id: "bug-expect".to_string(), + pattern: r#"\.expect\(""# + .to_string(), + severity: Severity::Minor, + 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()], + }, + CustomRule { + id: "bug-println".to_string(), + pattern: r"(?:println!|dbg!|print!\s*\()".to_string(), + severity: Severity::Minor, + 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()], + }, + CustomRule { + id: "bug-todo".to_string(), + pattern: r"(?i)\b(?:TODO|FIXME|HACK|XXX)\b".to_string(), + severity: Severity::Info, + message: "TODO/FIXME/HACK/XXX comment found. Consider resolving before merge.".to_string(), + languages: vec!["all".to_string()], + exclude: vec![], + }, + CustomRule { + id: "bug-console-log".to_string(), + pattern: r"console\.(?:log|debug|info)\s*\(".to_string(), + severity: Severity::Minor, + 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()], + }, + CustomRule { + id: "bug-hardcoded-port".to_string(), + pattern: r#"(?::"8080"|:"3000"|:"5000")"#.to_string(), + severity: Severity::Info, + message: "Hardcoded port number detected. Consider using environment variables or config.".to_string(), + languages: vec!["all".to_string()], + exclude: vec![], + }, + // --- Quality --- + CustomRule { + id: "qual-error-ignore".to_string(), + pattern: r"let\s+_\s*=\s*\w+::\w+\s*\(".to_string(), + severity: Severity::Minor, + message: "Error result discarded with `let _ =`. Consider handling the error explicitly.".to_string(), + languages: vec!["all".to_string()], + exclude: vec![], + }, + CustomRule { + id: "qual-clone".to_string(), + pattern: r"\.clone\(\)".to_string(), + severity: Severity::Info, + message: "Use of `.clone()` detected. Consider borrowing or ownership transfer.".to_string(), + languages: vec!["rs".to_string()], + exclude: vec![], + }, + ] +} + +/// Post-match filter for rules that need additional validation after regex match. +/// Returns `true` to suppress a finding that the regex matched but should be ignored. +pub fn post_match_filter(rule_id: &str, line: &str) -> bool { + match rule_id { + "sec-hardcoded-url" => { + let lower = line.to_lowercase(); + lower.contains("http://localhost") + || lower.contains("http://127.0.0.1") + || lower.contains("http://0.0.0.0") + || lower.contains("http://[::1]") + } + _ => false, + } +} diff --git a/src/engine/rules/matching.rs b/src/engine/rules/matching.rs new file mode 100644 index 0000000..ad8d9ae --- /dev/null +++ b/src/engine/rules/matching.rs @@ -0,0 +1,193 @@ +/// Matching engine for rules against diff lines. +use tracing::debug; + +use super::types::CustomRule; + +/// Check if a rule applies to the given file language. +pub fn matches_language(rule: &CustomRule, file_lang: &str) -> bool { + rule.languages.iter().any(|lang| { + let lang_lower = lang.to_lowercase(); + lang_lower == "all" || lang_lower == file_lang + }) +} + +/// Check if a file path should be excluded from a rule based on glob patterns. +pub fn matches_exclude(rule: &CustomRule, file_path: &str) -> bool { + // Also check built-in default exclude paths + let all_excludes: Vec<&str> = rule + .exclude + .iter() + .map(String::as_str) + .chain(DEFAULT_EXCLUDE_PATHS.iter().copied()) + .collect(); + + for pattern in &all_excludes { + // Simple glob matching: treat * as any chars, support directory prefixes + if glob_matches(pattern, file_path) { + return true; + } + } + + false +} + +/// 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"); + false + } + } +} + +/// Simple glob matching: supports `*` wildcard and directory prefix checks. +fn glob_matches(pattern: &str, path: &str) -> bool { + // Handle "*_test.*" style patterns + if pattern.starts_with('*') && pattern.contains('.') { + // e.g., "*_test.*" — match files ending with "_test.something" + let suffix = &pattern[1..]; // "_test.*" + if let Some(dot_idx) = suffix.find('.') { + let name_part = &suffix[..dot_idx]; // "_test" + let ext_part = &suffix[dot_idx..]; // ".*" + let file_name = path.rsplit('/').next().unwrap_or(path); + if file_name.ends_with(name_part) { + let remaining = &file_name[file_name.len() - name_part.len()..]; + if remaining == name_part { + // Check extension + if ext_part == ".*" { + return true; + } + if let Some(file_ext) = file_name.rsplit_once(name_part).map(|(_, ext)| ext) { + if file_ext.starts_with('.') + && ext_part + .strip_prefix('.') + .is_some_and(|e| file_ext.ends_with(e)) + { + return true; + } + } + } + } + } + } + + // Handle "dir/" prefix patterns (directory-based exclusion) + if pattern.ends_with('/') { + return path.starts_with(pattern) || path.contains(&pattern.to_string()); + } + + // Handle "**" glob in patterns like "tests/**" + if pattern.contains("**") { + let prefix = pattern.split("**").next().unwrap_or(""); + return path.starts_with(prefix); + } + + // Handle simple substring patterns + if pattern.starts_with('*') && pattern.ends_with('*') { + let inner = &pattern[1..pattern.len() - 1]; + return path.contains(inner); + } + + // Exact or prefix match + path == pattern || path.starts_with(&format!("{pattern}/")) +} + +/// Default paths to exclude from rule matching (test directories, fixtures, etc.). +const DEFAULT_EXCLUDE_PATHS: &[&str] = &[ + "tests/", + "test/", + "__tests__/", + "spec/", + "fixtures/", + "examples/", + // File patterns handled in glob_matches: *_test.*, *_spec.* +]; + +#[cfg(test)] +mod tests { + use super::*; + use crate::engine::Severity; + + fn make_rule(pattern: &str, languages: &[&str], exclude: &[&str]) -> CustomRule { + 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(), + } + } + + #[test] + fn matches_language_all() { + let rule = make_rule("test", &["all"], &[]); + assert!(matches_language(&rule, "rs")); + assert!(matches_language(&rule, "py")); + assert!(matches_language(&rule, "js")); + } + + #[test] + fn matches_language_specific() { + let rule = make_rule("test", &["rs"], &[]); + assert!(matches_language(&rule, "rs")); + assert!(!matches_language(&rule, "py")); + } + + #[test] + fn matches_language_case_insensitive() { + let rule = make_rule("test", &["RS"], &[]); + assert!(matches_language(&rule, "rs")); + } + + #[test] + fn matches_exclude_test_dir() { + let rule = make_rule("test", &["all"], &[]); + assert!(matches_exclude(&rule, "tests/main.rs")); + assert!(matches_exclude(&rule, "test/helper.py")); + } + + #[test] + fn not_excluded_src_dir() { + let rule = make_rule("test", &["all"], &[]); + assert!(!matches_exclude(&rule, "src/main.rs")); + } + + #[test] + fn matches_exclude_custom() { + let rule = make_rule("test", &["all"], &["vendor/"]); + assert!(matches_exclude(&rule, "vendor/lib.rs")); + } + + #[test] + fn match_rule_against_line_simple() { + let rule = make_rule(r"println!", &["all"], &[]); + assert!(match_rule_against_line(&rule, "println!(\"hello\")")); + assert!(!match_rule_against_line(&rule, "let x = 1;")); + } + + #[test] + fn match_rule_invalid_regex_returns_false() { + let rule = make_rule(r"(?P Vec { + if !config.enabled { + debug!("rule engine is disabled"); + return Vec::new(); + } + + let mut all_rules = builtin::builtin_rules(); + all_rules.extend(config.custom_rules.clone()); + + debug!( + rule_count = all_rules.len(), + "running rules against diff chunks" + ); + + let mut findings = Vec::new(); + + for file in chunks { + let file_path = file + .new_path + .as_deref() + .or(file.old_path.as_deref()) + .unwrap_or("unknown"); + + for hunk in &file.chunks { + for line in &hunk.lines { + // Only check added lines (new code being introduced) + if line.line_type != DiffLineType::Add { + continue; + } + + for rule in &all_rules { + // Check language filter + if !matching::matches_language(rule, &file.language) { + continue; + } + + // Check exclude filter + if matching::matches_exclude(rule, file_path) { + continue; + } + + // Check pattern match + if !matching::match_rule_against_line(rule, &line.content) { + continue; + } + + // Post-match filter (e.g., allow localhost URLs) + if builtin::post_match_filter(&rule.id, &line.content) { + continue; + } + + let line_no = line.new_line_no.unwrap_or(0); + + findings.push(RuleFinding { + rule_id: rule.id.clone(), + file: file_path.to_string(), + line: line_no, + severity: rule.severity, + title: format!("[{}] Rule: {}", rule.id, rule.id), + body: rule.message.clone(), + }); + } + } + } + } + + // Sort by severity (Critical first) then by file + line + findings.sort_by(|a, b| { + a.severity + .cmp(&b.severity) + .then_with(|| a.file.cmp(&b.file)) + .then_with(|| a.line.cmp(&b.line)) + }); + + // Deduplicate: same rule_id + file + line + findings.dedup_by(|a, b| a.rule_id == b.rule_id && a.file == b.file && a.line == b.line); + + // Cap at max_findings + let capped = findings.len().min(config.max_findings); + if findings.len() > capped { + debug!(total = findings.len(), capped, "capping rule findings"); + findings.truncate(capped); + } + + debug!(findings = findings.len(), "rule engine complete"); + findings +} + +/// Merge rule-based findings with LLM-produced issues into a single list. +/// +/// Rule findings are appended after LLM issues (LLM issues take priority). +/// Duplicates (same file + line) from rules are skipped if the LLM already +/// reported an issue for that location. +pub fn merge_rule_findings( + llm_issues: Vec, + rule_findings: Vec, +) -> Vec { + let mut result = llm_issues; + + // Build a set of (file, line) pairs from LLM issues to avoid duplicates + let llm_locations: std::collections::HashSet<(String, u32)> = result + .iter() + .filter_map(|issue| issue.line.map(|ln| (issue.file.clone(), ln))) + .collect(); + + for finding in rule_findings { + // Skip if LLM already has an issue at the same file+line + if llm_locations.contains(&(finding.file.clone(), finding.line)) { + debug!( + rule_id = %finding.rule_id, + file = %finding.file, + line = finding.line, + "skipping rule finding (LLM already reported issue at this location)" + ); + continue; + } + + result.push(ReviewIssue { + file: finding.file, + line: Some(finding.line), + severity: finding.severity, + issue_type: Some("rule".to_string()), + title: finding.title, + body: finding.body, + suggested_fix: None, + }); + } + + result +} + +/// Format rule findings as a context string for injection into the LLM prompt. +pub fn format_rule_context(findings: &[RuleFinding]) -> String { + if findings.is_empty() { + return String::new(); + } + + let mut ctx = String::from("Static rule engine findings (pre-verified):\n"); + ctx.push_str("---\n"); + + for f in findings { + ctx.push_str(&format!( + "- [{}] {}:{} — {} ({}): {}", + f.severity, f.file, f.line, f.rule_id, f.severity, f.body + )); + ctx.push('\n'); + } + + ctx.push_str("---\n"); + ctx +} + +/// Convenience: parse diff + run rules in one step. +#[allow(dead_code)] // public API for future standalone usage +pub fn parse_and_run_rules(diff: &str, config: &RulesConfig) -> Vec { + let chunks = parse_diff(diff); + run_rules(&chunks, config) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::engine::Severity; + + fn default_config() -> RulesConfig { + RulesConfig { + enabled: true, + max_findings: 10, + custom_rules: Vec::new(), + } + } + + #[test] + fn unwrap_rule_fires_on_rust_code() { + let diff = r#"diff --git a/src/main.rs b/src/main.rs +--- a/src/main.rs ++++ b/src/main.rs +@@ -1,2 +1,3 @@ + fn main() { ++ let x = something.unwrap(); + } +"#; + let findings = parse_and_run_rules(diff, &default_config()); + let unwrap_findings: Vec<_> = findings + .iter() + .filter(|f| f.rule_id == "bug-unwrap") + .collect(); + assert!( + !unwrap_findings.is_empty(), + "should detect .unwrap() in added Rust code" + ); + } + + #[test] + fn todo_rule_fires() { + let diff = r#"diff --git a/src/main.rs b/src/main.rs +--- a/src/main.rs ++++ b/src/main.rs +@@ -1,2 +1,3 @@ + fn main() { ++ // TODO: fix this later + } +"#; + let findings = parse_and_run_rules(diff, &default_config()); + let todo_findings: Vec<_> = findings + .iter() + .filter(|f| f.rule_id == "bug-todo") + .collect(); + assert!(!todo_findings.is_empty(), "should detect TODO comment"); + } + + #[test] + fn println_rule_fires_in_rust() { + let diff = r#"diff --git a/src/lib.rs b/src/lib.rs +--- a/src/lib.rs ++++ b/src/lib.rs +@@ -1,1 +1,2 @@ + pub fn greet() { ++ println!("hello"); + } +"#; + let findings = parse_and_run_rules(diff, &default_config()); + assert!( + findings.iter().any(|f| f.rule_id == "bug-println"), + "should detect println! macro" + ); + } + + #[test] + fn console_log_rule_fires_in_ts() { + let diff = r#"diff --git a/app.ts b/app.ts +--- a/app.ts ++++ b/app.ts +@@ -1,1 +1,2 @@ + function init() { ++ console.log("starting"); + } +"#; + let findings = parse_and_run_rules(diff, &default_config()); + assert!( + findings.iter().any(|f| f.rule_id == "bug-console-log"), + "should detect console.log in TypeScript" + ); + } + + #[test] + fn language_filter_works() { + // println! in a Python file should NOT fire bug-println (Rust only) + let diff = r#"diff --git a/script.py b/script.py +--- a/script.py ++++ b/script.py +@@ -1,1 +1,2 @@ + def main(): ++ println!("hello") +"#; + let findings = parse_and_run_rules(diff, &default_config()); + assert!( + !findings.iter().any(|f| f.rule_id == "bug-println"), + "bug-println should not fire on Python files" + ); + } + + #[test] + fn exclude_filter_works() { + // unwrap in test/ directory should NOT fire + let diff = r#"diff --git a/tests/integration.rs b/tests/integration.rs +--- a/tests/integration.rs ++++ b/tests/integration.rs +@@ -1,1 +1,2 @@ + #[test] ++fn test_something() { let _ = result.unwrap(); } +"#; + let findings = parse_and_run_rules(diff, &default_config()); + assert!( + !findings.iter().any(|f| f.rule_id == "bug-unwrap"), + "bug-unwrap should not fire in tests/ directory" + ); + } + + #[test] + fn disabled_config_returns_no_findings() { + let diff = r#"diff --git a/src/main.rs b/src/main.rs +--- a/src/main.rs ++++ b/src/main.rs +@@ -1,2 +1,3 @@ + fn main() { ++ let x = something.unwrap(); + } +"#; + let config = RulesConfig { + enabled: false, + ..default_config() + }; + let findings = parse_and_run_rules(diff, &config); + assert!( + findings.is_empty(), + "disabled config should produce no findings" + ); + } + + #[test] + fn max_findings_cap() { + let diff = r#"diff --git a/src/main.rs b/src/main.rs +--- a/src/main.rs ++++ b/src/main.rs +@@ -1,2 +1,8 @@ + fn main() { ++ let a = x.unwrap(); ++ let b = y.clone(); ++ let c = z.clone(); ++ println!("debug"); ++ // TODO: something ++ // FIXME: another + } +"#; + let config = RulesConfig { + max_findings: 2, + ..default_config() + }; + let findings = parse_and_run_rules(diff, &config); + assert!(findings.len() <= 2, "should cap findings at max_findings"); + } + + #[test] + fn merge_skips_duplicates() { + let llm = vec![ReviewIssue { + file: "src/main.rs".to_string(), + line: Some(5), + severity: Severity::Minor, + issue_type: Some("bug".to_string()), + title: "Unnecessary unwrap".to_string(), + body: "Use proper error handling".to_string(), + suggested_fix: None, + }]; + + let rules = vec![RuleFinding { + rule_id: "bug-unwrap".to_string(), + file: "src/main.rs".to_string(), + line: 5, + severity: Severity::Minor, + title: "[bug-unwrap] Rule: bug-unwrap".to_string(), + body: "Can panic".to_string(), + }]; + + let merged = merge_rule_findings(llm, rules); + // Should have 1 issue (LLM's), not 2 — rule finding at same location skipped + assert_eq!(merged.len(), 1); + } + + #[test] + fn merge_appends_unique_rule_findings() { + let llm = vec![ReviewIssue { + file: "src/main.rs".to_string(), + line: Some(5), + severity: Severity::Minor, + issue_type: Some("bug".to_string()), + title: "Some issue".to_string(), + body: "Details".to_string(), + suggested_fix: None, + }]; + + let rules = vec![RuleFinding { + rule_id: "bug-todo".to_string(), + file: "src/main.rs".to_string(), + line: 10, + severity: Severity::Info, + title: "[bug-todo] Rule: bug-todo".to_string(), + body: "TODO found".to_string(), + }]; + + let merged = merge_rule_findings(llm, rules); + assert_eq!(merged.len(), 2); + assert_eq!(merged[1].issue_type.as_deref(), Some("rule")); + } + + #[test] + fn format_rule_context_non_empty() { + let findings = vec![RuleFinding { + rule_id: "bug-todo".to_string(), + file: "src/lib.rs".to_string(), + line: 42, + severity: Severity::Info, + title: "[bug-todo] Rule: bug-todo".to_string(), + body: "TODO comment found".to_string(), + }]; + let ctx = format_rule_context(&findings); + assert!(ctx.contains("bug-todo")); + assert!(ctx.contains("src/lib.rs:42")); + } + + #[test] + fn format_rule_context_empty() { + let ctx = format_rule_context(&[]); + assert!(ctx.is_empty()); + } + + #[test] + fn hardcoded_secret_rule_fires() { + let diff = r#"diff --git a/config.py b/config.py +--- a/config.py ++++ b/config.py +@@ -1,2 +1,3 @@ + # Config ++password = "super_secret_123" + DB_HOST = "localhost" +"#; + let findings = parse_and_run_rules(diff, &default_config()); + assert!( + findings.iter().any(|f| f.rule_id == "sec-hardcoded-secret"), + "should detect hardcoded password" + ); + } + + #[test] + fn hardcoded_url_rule_fires() { + let diff = r#"diff --git a/src/client.rs b/src/client.rs +--- a/src/client.rs ++++ b/src/client.rs +@@ -1,2 +1,3 @@ + fn get_url() -> &'static str { ++ "http://example.com/api/data" + } +"#; + let chunks = parse_diff(diff); + eprintln!("DEBUG: {} chunks parsed", chunks.len()); + for (i, c) in chunks.iter().enumerate() { + eprintln!( + "DEBUG chunk[{}]: lang={}, new_path={:?}, hunks={}", + i, + c.language, + c.new_path, + c.chunks.len() + ); + for (j, h) in c.chunks.iter().enumerate() { + eprintln!("DEBUG hunk[{}]: {} lines", j, h.lines.len()); + for l in &h.lines { + eprintln!("DEBUG line: {:?} {:?}", l.line_type, l.content); + } + } + } + let findings = parse_and_run_rules(diff, &default_config()); + eprintln!("DEBUG: {} findings total", findings.len()); + for f in &findings { + eprintln!("DEBUG finding: {:?}", f); + } + assert!( + findings.iter().any(|f| f.rule_id == "sec-hardcoded-url"), + "should detect non-localhost http:// URL" + ); + } + + #[test] + fn hardcoded_url_localhost_allowed() { + let diff = r#"diff --git a/src/client.rs b/src/client.rs +--- a/src/client.rs ++++ b/src/client.rs +@@ -1,2 +1,3 @@ + fn get_url() -> &'static str { ++ "http://localhost:3000/health" + } +"#; + let findings = parse_and_run_rules(diff, &default_config()); + assert!( + !findings.iter().any(|f| f.rule_id == "sec-hardcoded-url"), + "localhost http URLs should be allowed" + ); + } +} diff --git a/src/engine/rules/types.rs b/src/engine/rules/types.rs new file mode 100644 index 0000000..dae6d8d --- /dev/null +++ b/src/engine/rules/types.rs @@ -0,0 +1,59 @@ +/// Rule types used by the rule engine. +use serde::{Deserialize, Serialize}; + +use crate::engine::Severity; + +/// Runtime configuration for the rule engine (lives on `Config`). +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct RulesConfig { + /// Whether the rule engine is enabled. + pub enabled: bool, + /// Maximum number of findings to report per review (prevents noisy output). + pub max_findings: usize, + /// User-defined custom rules, merged with built-in rules. + pub custom_rules: Vec, +} + +impl Default for RulesConfig { + fn default() -> Self { + Self { + enabled: true, + max_findings: 5, + custom_rules: Vec::new(), + } + } +} + +/// A user-defined or built-in rule definition. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct CustomRule { + /// Unique rule identifier (e.g., `"sec-hardcoded-secret"`). + pub id: String, + /// Regex pattern to match against diff lines. + pub pattern: String, + /// Severity of findings from this rule. + pub severity: Severity, + /// Human-readable description used as the finding body. + pub message: String, + /// Languages this rule applies to. `["all"]` means all languages. + pub languages: Vec, + /// Glob patterns for file paths to exclude from this rule. + pub exclude: Vec, +} + +/// A single finding produced by a rule. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct RuleFinding { + /// The rule that produced this finding. + pub rule_id: String, + /// File path where the finding was located. + pub file: String, + /// Line number in the new file. + pub line: u32, + /// Severity of this finding. + pub severity: Severity, + /// Short title. + pub title: String, + /// Detailed description. + pub body: String, +} diff --git a/src/formatters/sarif.rs b/src/formatters/sarif.rs index 837fb6f..24577e0 100644 --- a/src/formatters/sarif.rs +++ b/src/formatters/sarif.rs @@ -111,7 +111,7 @@ fn build_sarif(issues: &[ReviewIssue]) -> Value { json!([{ "executionSuccessful": true, "properties": { - "cora.watermark": format!("Reviewed by Cora v{}", env!("CARGO_PKG_VERSION")) + "cora.watermark": format!("Reviewed by CodeCora v{}", env!("CARGO_PKG_VERSION")) } }]) }; @@ -122,7 +122,8 @@ fn build_sarif(issues: &[ReviewIssue]) -> Value { "runs": [{ "tool": { "driver": { - "name": "Cora", + "name": "CodeCora", + "fullName": "codecoradev/cora-cli", "version": env!("CARGO_PKG_VERSION"), "informationUri": env!("CARGO_PKG_REPOSITORY"), "rules": rules @@ -212,7 +213,7 @@ mod tests { } #[test] - fn sarif_tool_driver_name_is_cora() { + fn sarif_tool_driver_name_is_codecora() { let fmt = SarifFormatter; let output = fmt.format_review(&sample_response()).unwrap(); let parsed: serde_json::Value = serde_json::from_str(&output).unwrap(); @@ -220,7 +221,20 @@ mod tests { parsed["runs"][0]["tool"]["driver"]["name"] .as_str() .unwrap(), - "Cora" + "CodeCora" + ); + } + + #[test] + fn sarif_tool_driver_has_fullname_codecoradev() { + let fmt = SarifFormatter; + let output = fmt.format_review(&sample_response()).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&output).unwrap(); + assert_eq!( + parsed["runs"][0]["tool"]["driver"]["fullName"] + .as_str() + .unwrap(), + "codecoradev/cora-cli" ); }