diff --git a/CHANGELOG.md b/CHANGELOG.md index 42d0003..26d98b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added — Deeper, token-economical cross-file review context + +- **Inbound caller (blast-radius) resolution.** Reviews now also resolve **who calls the changed code**, not just what the changed code calls. A changed function/type signature surfaces its call-sites across the repo so breaking changes can be flagged. Gated by new `review.context_chain.include_callers` (default `true`); uses gitignore-aware walking and is bounded (≤400 files, ≤3 call-sites/symbol), injecting only the call line + 1 line of context to stay cheap. +- **Definition extraction** (`extract_definitions_from_diff`) for Rust/Python/JS-TS/Go/Java-Kotlin — detects functions/types *declared* in the diff, which feed caller resolution. +- **Signature-only budget fallback.** When the token budget can't fit a full function/type body, a thin signature slice is injected instead of skipping the entry entirely (~3-5× more symbols under the same budget). + +### Fixed — Cross-file context correctness & defaults + +- **`review.rs` passed the wrong ignore list** to the context resolver (`ignore.rules` — finding-type strings — instead of `ignore.files` globs). The resolver could inject code from `node_modules/`/`target/`. Now uses `ignore.files`. +- **`context_chain.max_context_tokens` default raised 3000 → 5000**, and the resolver never scans gitignored build artifacts (caller scan uses the `ignore` crate). + ### Fixed — Minor Best-Practice Items (#334) - **Test-file detection no longer over-matches (#87).** `is_test_file` now uses path-segment awareness, so common words like `latest`, `aspect`, `attestation`, `protest` are no longer mistaken for test files. diff --git a/docs/changelog.md b/docs/changelog.md index b1972b1..8122537 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added — Deeper cross-file review context (token-economical) + +- **Caller/blast-radius resolution**: reviews now resolve who *calls* changed code (not just what changed code calls). `review.context_chain.include_callers` (default `true`); bounded scan, thin slices. +- **Definition extraction** (Rust/Python/JS-TS/Go/Java-Kotlin) feeds caller resolution. +- **Signature-only budget fallback**: injects a signature slice when the full body won't fit. + +### Fixed + +- `review.rs` passed `ignore.rules` instead of `ignore.files` to the context resolver (could inject `node_modules`/`target` code). +- `context_chain.max_context_tokens` default 3000 → 5000. + ### Fixed — Minor Best-Practice Items (#334) - **#87** test-file detection uses path-segment awareness (no more `latest`/`aspect`/`attestation` false matches). diff --git a/src/engine/context/extraction.rs b/src/engine/context/extraction.rs index 5501eff..2bc332c 100644 --- a/src/engine/context/extraction.rs +++ b/src/engine/context/extraction.rs @@ -75,6 +75,25 @@ static RE_JAVA_TYPE: LazyLock = /// Maximum number of symbols to extract per file to prevent runaway extraction. const MAX_SYMBOLS_PER_FILE: usize = 50; +// ─── Definition regexes (functions/types *declared* in changed lines) ─── +// Used for inbound caller (blast-radius) resolution. + +static RE_DEF_RUST_FN: LazyLock = + LazyLock::new(|| Regex::new(r"\b(?:pub\s+)?(?:async\s+)?(?:unsafe\s+)?fn\s+(\w+)").unwrap()); +static RE_DEF_RUST_TYPE: LazyLock = + LazyLock::new(|| Regex::new(r"\b(?:pub\s+)?(?:struct|enum|trait|type)\s+(\w+)").unwrap()); +static RE_DEF_PY_FN: LazyLock = LazyLock::new(|| Regex::new(r"\bdef\s+(\w+)").unwrap()); +static RE_DEF_PY_TYPE: LazyLock = LazyLock::new(|| Regex::new(r"\bclass\s+(\w+)").unwrap()); +static RE_DEF_JS_FN: LazyLock = + LazyLock::new(|| Regex::new(r"\bfunction\s+\*?\s*(\w+)").unwrap()); +static RE_DEF_JS_TYPE: LazyLock = LazyLock::new(|| Regex::new(r"\bclass\s+(\w+)").unwrap()); +static RE_DEF_GO_FN: LazyLock = + LazyLock::new(|| Regex::new(r"\bfunc\s+(?:\([^)]*\)\s*)?(\w+)\s*\(").unwrap()); +static RE_DEF_GO_TYPE: LazyLock = + LazyLock::new(|| Regex::new(r"\btype\s+(\w+)\s+(?:struct|interface)").unwrap()); +static RE_DEF_JAVA_TYPE: LazyLock = + LazyLock::new(|| Regex::new(r"\b(?:class|interface|enum)\s+(\w+)").unwrap()); + /// Extract symbols from a single line of code for a given language. pub fn extract_symbols_from_line(line: &str, language: &str) -> Vec { let mut symbols = Vec::new(); @@ -331,6 +350,85 @@ pub fn extract_symbols_from_diff( all_symbols } +/// Extract function/type *definitions* declared in the added lines of a diff. +/// These are the symbols whose **callers** (blast radius) we want to resolve. +/// +/// Only added lines are scanned (a definition only matters if this PR +/// introduces or modifies it). Returns deduplicated `(name, kind, file)`. +pub fn extract_definitions_from_diff( + chunks: &[crate::engine::diff_parser::FileChunk], +) -> Vec { + use crate::engine::context::types::{DefinedSymbol, DefinitionKind}; + use crate::engine::diff_parser::DiffLineType; + let mut out = Vec::new(); + let mut seen: HashSet<(String, String)> = HashSet::new(); // (name, file) + + for file in chunks { + if file.is_binary || file.is_deleted { + continue; + } + let path = file + .new_path + .as_deref() + .or(file.old_path.as_deref()) + .unwrap_or("unknown"); + let lang = &file.language; + + for hunk in &file.chunks { + for line in &hunk.lines { + if line.line_type != DiffLineType::Add { + continue; + } + let content = &line.content; + let (fn_re, type_re): (Option<&Regex>, Option<&Regex>) = match lang.as_str() { + "rs" => (Some(&RE_DEF_RUST_FN), Some(&RE_DEF_RUST_TYPE)), + "py" | "pyi" => (Some(&RE_DEF_PY_FN), Some(&RE_DEF_PY_TYPE)), + "ts" | "tsx" | "js" | "jsx" | "mjs" | "cjs" => { + (Some(&RE_DEF_JS_FN), Some(&RE_DEF_JS_TYPE)) + } + "go" => (Some(&RE_DEF_GO_FN), Some(&RE_DEF_GO_TYPE)), + "java" | "kt" | "kts" => (None, Some(&RE_DEF_JAVA_TYPE)), + _ => (None, None), + }; + + let mut push = |name: &str, kind: DefinitionKind| { + if name.is_empty() { + return; + } + // Skip obvious noise / keywords that slip through. + if matches!( + name, + "if" | "for" | "while" | "match" | "new" | "return" | "test" | "main" + ) { + return; + } + if seen.insert((name.to_string(), path.to_string())) { + out.push(DefinedSymbol { + name: name.to_string(), + kind, + file: path.to_string(), + }); + } + }; + + if let Some(re) = fn_re { + for cap in re.captures_iter(content) { + push(&cap[1], DefinitionKind::Function); + } + } + if let Some(re) = type_re { + for cap in re.captures_iter(content) { + push(&cap[1], DefinitionKind::Type); + } + } + } + } + } + + debug!(definitions = out.len(), "extracted definitions from diff"); + out +} + #[cfg(test)] mod tests { use super::*; @@ -365,6 +463,63 @@ mod tests { // --- Rust extraction --- + #[test] + fn extract_definitions_rust_fn_and_type() { + let chunks = vec![make_file_chunk( + "src/lib.rs", + "rs", + &[ + ("pub fn validate_token(token: &str) -> bool {", 1), + ("pub struct CryptoConfig { seed: u64 }", 2), + ], + )]; + let defs = extract_definitions_from_diff(&chunks); + let names: Vec<&str> = defs.iter().map(|d| d.name.as_str()).collect(); + assert!( + names.contains(&"validate_token"), + "should detect fn definition" + ); + assert!( + names.contains(&"CryptoConfig"), + "should detect struct definition" + ); + } + + #[test] + fn extract_definitions_python() { + let chunks = vec![make_file_chunk( + "app/auth.py", + "py", + &[("def login(user):", 1), ("class User:", 2)], + )]; + let defs = extract_definitions_from_diff(&chunks); + let names: Vec<&str> = defs.iter().map(|d| d.name.as_str()).collect(); + assert!(names.contains(&"login")); + assert!(names.contains(&"User")); + } + + #[test] + fn extract_definitions_go() { + let chunks = vec![make_file_chunk( + "main.go", + "go", + &[("func Parse(input string) error {", 1)], + )]; + let defs = extract_definitions_from_diff(&chunks); + assert!(defs.iter().any(|d| d.name == "Parse")); + } + + #[test] + fn extract_definitions_dedups_within_file() { + let chunks = vec![make_file_chunk( + "src/lib.rs", + "rs", + &[("fn helper() {}", 1), ("fn helper() {}", 2)], + )]; + let defs = extract_definitions_from_diff(&chunks); + assert_eq!(defs.iter().filter(|d| d.name == "helper").count(), 1); + } + #[test] fn extract_rust_function_calls() { let chunks = vec![make_file_chunk( diff --git a/src/engine/context/mod.rs b/src/engine/context/mod.rs index ff87282..752de90 100644 --- a/src/engine/context/mod.rs +++ b/src/engine/context/mod.rs @@ -48,15 +48,17 @@ pub fn build_context_chain( project_root: &Path, ignore_patterns: &[String], ) -> ContextChain { - // Step 1: Extract symbols from changed lines + // Step 1: Extract outbound symbols (what changed code calls/imports) let symbols = extraction::extract_symbols_from_diff(chunks); + // Step 1b: Extract inbound definitions (what changed code defines — for callers) + let defs = extraction::extract_definitions_from_diff(chunks); - if symbols.is_empty() || !config.enabled { + if (symbols.is_empty() && defs.is_empty()) || !config.enabled { return ContextChain::default(); } // Step 2: Resolve and assemble under budget - resolver::build_context_chain(&symbols, config, project_root, ignore_patterns) + resolver::build_context_chain(&symbols, &defs, config, project_root, ignore_patterns) } #[cfg(test)] diff --git a/src/engine/context/resolver.rs b/src/engine/context/resolver.rs index 1ae197a..18b72db 100644 --- a/src/engine/context/resolver.rs +++ b/src/engine/context/resolver.rs @@ -13,11 +13,12 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; +use regex::Regex; use tracing::debug; use super::types::{ - ContextChain, ContextConfig, ContextEntry, ContextPriority, ContextStats, ExtractedSymbol, - SymbolKind, estimate_tokens, + ContextChain, ContextConfig, ContextEntry, ContextPriority, ContextStats, DefinedSymbol, + DefinitionKind, ExtractedSymbol, SymbolKind, estimate_tokens, }; /// Maximum lines to read per symbol definition (prevents reading huge functions). @@ -26,6 +27,16 @@ const MAX_FN_LINES: usize = 50; const MAX_TYPE_LINES: usize = 50; /// Maximum lines to read per test function. const MAX_TEST_LINES: usize = 30; +/// Lines to read per caller (blast-radius) call-site: the call line + context. +const MAX_CALLER_LINES: usize = 4; +/// Maximum number of source files to scan when resolving callers. +const MAX_CALLER_FILES_SCAN: usize = 400; +/// Maximum call-sites injected per changed symbol (keeps callers token-cheap). +const MAX_CALLERS_PER_SYMBOL: usize = 3; +/// Source extensions scanned for caller resolution. +const CALLER_SCAN_EXTS: &[&str] = &[ + "rs", "py", "pyi", "ts", "tsx", "js", "jsx", "mjs", "cjs", "go", "java", "kt", "kts", +]; /// Safely join a relative path with a project root, verifying that the /// resulting path stays within the project root (prevents path traversal). @@ -60,11 +71,12 @@ fn safe_join(project_root: &Path, relative: &str) -> Option { /// This is the main entry point: extract → resolve → read → budget → assemble. pub fn build_context_chain( symbols: &[ExtractedSymbol], + defs: &[DefinedSymbol], config: &ContextConfig, project_root: &Path, ignore_patterns: &[String], ) -> ContextChain { - if !config.enabled || symbols.is_empty() { + if !config.enabled || (symbols.is_empty() && defs.is_empty()) { return ContextChain::default(); } @@ -73,7 +85,7 @@ pub fn build_context_chain( ..Default::default() }; - // Phase 1: Resolve symbols to file locations + // Phase 1: Resolve outbound symbols to file locations (what changed code calls) let mut entries = resolve_symbols(symbols, config, project_root, ignore_patterns, &mut stats); // Phase 2: Add test file mappings @@ -81,21 +93,36 @@ pub fn build_context_chain( add_test_mappings(symbols, project_root, &mut entries, &mut stats); } - // Sort by priority (FunctionDef first, Test last) + // Phase 3: Resolve inbound callers (blast radius — who calls changed code) + entries.extend(resolve_callers(defs, config, project_root, ignore_patterns)); + + // Sort by priority (FunctionDef first, CallerSite last) entries.sort_by_key(|e| e.priority); - // Phase 3: Read file content under budget + // Phase 4: 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 { + // Tier 3: signature-only fallback — inject a thin slice instead of + // skipping the entry entirely. Keeps high-value defs under budget. + if let Some(sig) = signature_only(entry, project_root) { + let sig_tokens = estimate_tokens(&sig); + if sig_tokens > 0 && sig_tokens <= budget { + budget -= sig_tokens; + stats.entries_read += 1; + stats.estimated_tokens += sig_tokens; + parts.push(format!( + "--- {}:{}-{} ({}, signature only) ---\n{}", + entry.file, entry.line_start, entry.line_end, entry.label, sig + )); + continue; + } + } stats.budget_hit = true; debug!( entry = %entry.label, @@ -106,6 +133,10 @@ pub fn build_context_chain( continue; } + if content.is_empty() { + continue; + } + budget -= tokens; stats.entries_read += 1; stats.estimated_tokens += tokens; @@ -678,6 +709,141 @@ fn find_go_package_file(dir: &Path, import_path: &str) -> Option { } /// Check if a file path matches any ignore pattern. +/// Resolve **callers** (blast radius) of functions/types defined in the diff. +/// +/// This is the inbound counterpart to outbound symbol resolution: instead of +/// "what does the changed code call", it answers "who calls the changed code" — +/// the most valuable context for flagging breaking signature/type changes. +/// +/// Uses gitignore-aware walking (the `ignore` crate) so build artifacts are +/// never scanned, and is bounded by [`MAX_CALLER_FILES_SCAN`] files and +/// [`MAX_CALLERS_PER_SYMBOL`] call-sites per symbol. Caller slices are tiny +/// (the call line + 1 line of context) to stay token-economical. +fn resolve_callers( + defs: &[DefinedSymbol], + config: &ContextConfig, + project_root: &Path, + ignore_patterns: &[String], +) -> Vec { + if !config.include_callers || defs.is_empty() { + return Vec::new(); + } + + // Precompile a matcher per definition. Skip names that are too short/noisy. + let matchers: Vec<(&DefinedSymbol, Regex)> = defs + .iter() + .filter_map(|d| { + if d.name.len() < 2 { + return None; + } + let pattern = match d.kind { + DefinitionKind::Function => format!(r"\b{}\s*\(", regex::escape(&d.name)), + DefinitionKind::Type => format!(r"\b{}\b", regex::escape(&d.name)), + }; + Regex::new(&pattern).ok().map(|r| (d, r)) + }) + .collect(); + if matchers.is_empty() { + return Vec::new(); + } + + let walker = ignore::WalkBuilder::new(project_root) + .hidden(true) + .git_ignore(true) + .git_global(true) + .git_exclude(true) + .build(); + + let mut entries = Vec::new(); + let mut files_scanned = 0usize; + + for dent in walker { + if files_scanned >= MAX_CALLER_FILES_SCAN { + break; + } + let dent = match dent { + Ok(d) => d, + Err(_) => continue, + }; + if !dent.file_type().map(|t| t.is_file()).unwrap_or(false) { + continue; + } + let path = dent.path(); + let ext = path.extension().and_then(|e| e.to_str()).unwrap_or(""); + if !CALLER_SCAN_EXTS.contains(&ext) { + continue; + } + + let rel = match path.strip_prefix(project_root) { + Ok(r) => r.to_string_lossy().replace('\\', "/").to_string(), + Err(_) => continue, + }; + if is_ignored(&rel, ignore_patterns) { + continue; + } + + let content = match std::fs::read_to_string(path) { + Ok(c) => c, + Err(_) => continue, + }; + files_scanned += 1; + + for (def, re) in &matchers { + // The defining file is not a "caller" of its own symbol. + if def.file == rel { + continue; + } + let mut hits = 0usize; + for (idx, line) in content.lines().enumerate() { + if hits >= MAX_CALLERS_PER_SYMBOL { + break; + } + if re.is_match(line) { + let ln = (idx + 1) as u32; + let label = match def.kind { + DefinitionKind::Function => format!("caller of fn {}", def.name), + DefinitionKind::Type => format!("usage of {}", def.name), + }; + entries.push(ContextEntry { + file: rel.clone(), + line_start: ln.saturating_sub(1).max(1), + line_end: ln + 1, + label, + priority: ContextPriority::CallerSite, + }); + hits += 1; + } + } + } + } + + debug!( + callers_found = entries.len(), + files_scanned, "resolved callers" + ); + entries +} + +/// Extract just the signature (not the full body) of a definition, for the +/// budget-aware fallback. Reads up to the opening `{` or a few lines. +fn signature_only(entry: &ContextEntry, project_root: &Path) -> Option { + let full = safe_join(project_root, &entry.file).filter(|p| p.exists())?; + let content = std::fs::read_to_string(&full).ok()?; + let start = entry.line_start.saturating_sub(1) as usize; + let mut sig: Vec<&str> = Vec::new(); + for line in content.lines().skip(start).take(8) { + sig.push(line); + if line.contains('{') || sig.len() >= 4 { + break; + } + } + if sig.is_empty() { + None + } else { + Some(sig.join("\n")) + } +} + fn is_ignored(file: &str, patterns: &[String]) -> bool { for pattern in patterns { // Simple glob-like matching: check if file contains the pattern @@ -825,6 +991,7 @@ fn read_entry_content(entry: &ContextEntry, project_root: &Path) -> String { ContextPriority::FunctionDef => MAX_FN_LINES, ContextPriority::TypeDef => MAX_TYPE_LINES, ContextPriority::Test => MAX_TEST_LINES, + ContextPriority::CallerSite => MAX_CALLER_LINES, }; if result.lines().count() > max_lines { @@ -964,14 +1131,14 @@ mod tests { enabled: false, ..Default::default() }; - let chain = build_context_chain(&[], &config, Path::new("/tmp"), &[]); + 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"), &[]); + let chain = build_context_chain(&[], &[], &config, Path::new("/tmp"), &[]); assert!(chain.text.is_empty()); } @@ -983,6 +1150,7 @@ mod tests { max_context_tokens: 5, // very small follow_depth: 1, include_tests: false, + include_callers: false, }; let symbols = vec![ExtractedSymbol { @@ -992,7 +1160,7 @@ mod tests { raw: "some_func()".to_string(), }]; - let chain = build_context_chain(&symbols, &config, Path::new("/tmp"), &[]); + 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); } @@ -1006,6 +1174,7 @@ mod tests { max_context_tokens: 100, follow_depth: 1, include_tests: false, + include_callers: false, }; let symbols = vec![ExtractedSymbol { @@ -1015,9 +1184,94 @@ mod tests { raw: "use crate::engine::scanner;".to_string(), }]; - let chain = build_context_chain(&symbols, &config, Path::new("/tmp"), &[]); + 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); } + + // ─── caller (blast-radius) resolution ─── + + #[test] + fn resolve_callers_finds_call_site() { + use crate::engine::context::types::{DefinedSymbol, DefinitionKind}; + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + std::fs::create_dir_all(root.join("src")).unwrap(); + std::fs::write( + root.join("src/caller.rs"), + "fn caller() {\n validate_token(t);\n}\n", + ) + .unwrap(); + + let defs = vec![DefinedSymbol { + name: "validate_token".to_string(), + kind: DefinitionKind::Function, + file: "src/auth.rs".to_string(), + }]; + let entries = resolve_callers(&defs, &ContextConfig::default(), root, &[]); + assert!( + entries + .iter() + .any(|e| e.file == "src/caller.rs" && e.label.contains("validate_token")), + "should resolve the caller site: {entries:?}" + ); + } + + #[test] + fn resolve_callers_skips_defining_file_and_respects_disable() { + use crate::engine::context::types::{DefinedSymbol, DefinitionKind}; + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + std::fs::create_dir_all(root.join("src")).unwrap(); + // The defining file itself contains the call — it must not self-report. + std::fs::write( + root.join("src/auth.rs"), + "fn auth() { validate_token(t); }\n", + ) + .unwrap(); + + let defs = vec![DefinedSymbol { + name: "validate_token".to_string(), + kind: DefinitionKind::Function, + file: "src/auth.rs".to_string(), + }]; + + // include_callers = true but only the defining file matches → no entries. + let entries = resolve_callers(&defs, &ContextConfig::default(), root, &[]); + assert!( + entries.is_empty(), + "defining file must not be its own caller" + ); + + // include_callers = false → no entries regardless. + let cfg = ContextConfig { + include_callers: false, + ..Default::default() + }; + assert!(resolve_callers(&defs, &cfg, root, &[]).is_empty()); + } + + // ─── signature-only fallback ─── + + #[test] + fn signature_only_returns_header_without_body() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + std::fs::write( + root.join("big.rs"), + "pub fn big(x: i32) -> i32 {\n let y = 1;\n y + x\n}\n", + ) + .unwrap(); + let entry = ContextEntry { + file: "big.rs".to_string(), + line_start: 1, + line_end: 4, + label: "fn big".to_string(), + priority: ContextPriority::FunctionDef, + }; + let sig = signature_only(&entry, root).expect("signature should be extracted"); + assert!(sig.contains("pub fn big"), "sig: {sig}"); + assert!(!sig.contains("y + x"), "body must not be included: {sig}"); + } } diff --git a/src/engine/context/types.rs b/src/engine/context/types.rs index 3bcca36..c3ff778 100644 --- a/src/engine/context/types.rs +++ b/src/engine/context/types.rs @@ -18,7 +18,7 @@ pub struct ContextConfig { /// 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. + /// Default: 5000 tokens ≈ 20 KB of code. #[serde(default = "default_max_context_tokens")] pub max_context_tokens: usize, @@ -34,6 +34,13 @@ pub struct ContextConfig { /// Default: true. #[serde(default = "default_true")] pub include_tests: bool, + + /// Whether to resolve **callers** of functions/types defined or modified in + /// the diff (inbound / blast-radius context). This is the highest-value + /// axis for deep review — it surfaces who depends on the changed code, so + /// breaking signature/type changes can be flagged. Default: true. + #[serde(default = "default_true")] + pub include_callers: bool, } fn default_true() -> bool { @@ -41,7 +48,7 @@ fn default_true() -> bool { } fn default_max_context_tokens() -> usize { - 3000 + 5000 } fn default_follow_depth() -> u32 { @@ -52,9 +59,10 @@ impl Default for ContextConfig { fn default() -> Self { Self { enabled: true, - max_context_tokens: 3000, + max_context_tokens: 5000, follow_depth: 1, include_tests: true, + include_callers: true, } } } @@ -95,6 +103,26 @@ pub struct ExtractedSymbol { pub raw: String, } +/// Kind of a symbol *defined* (declared) in the changed lines — used for +/// inbound caller/blast-radius resolution. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum DefinitionKind { + Function, + Type, +} + +/// A function/type *defined or modified* in the changed lines. These are the +/// symbols whose callers we want to find (blast radius). +#[derive(Debug, Clone)] +pub struct DefinedSymbol { + /// The defined symbol's name (e.g. `validate_token`, `CryptoConfig`). + pub name: String, + /// Whether it is a function/method or a type/class/struct. + pub kind: DefinitionKind, + /// The file where the definition appears. + pub file: String, +} + /// A resolved context entry — a (file, line range, symbol) tuple ready /// for reading and injection into the prompt. #[derive(Debug, Clone)] @@ -119,8 +147,10 @@ pub enum ContextPriority { FunctionDef = 0, /// Type/struct definitions. TypeDef = 1, - /// Test functions — lowest priority. + /// Test functions — lower priority. Test = 2, + /// Callers of changed code (blast-radius) — lowest, bonus context. + CallerSite = 3, } /// Statistics from a context chain build, useful for logging / progress events. @@ -169,9 +199,10 @@ mod tests { fn context_config_default() { let cfg = ContextConfig::default(); assert!(cfg.enabled); - assert_eq!(cfg.max_context_tokens, 3000); + assert_eq!(cfg.max_context_tokens, 5000); assert_eq!(cfg.follow_depth, 1); assert!(cfg.include_tests); + assert!(cfg.include_callers); } #[test] diff --git a/src/engine/review.rs b/src/engine/review.rs index 8820ec0..2be2f9d 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -185,11 +185,13 @@ async fn review_diff_inner( }; // Build context chain (cross-file dependency extraction) + // NOTE: pass ignore.files (e.g. target/**, node_modules/**) so the resolver + // never injects build-artifact code — not ignore.rules (finding-type strings). 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, + &config.ignore.files, ); let final_context = if !context_chain.text.is_empty() {