Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 11 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
155 changes: 155 additions & 0 deletions src/engine/context/extraction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,25 @@ static RE_JAVA_TYPE: LazyLock<Regex> =
/// 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<Regex> =
LazyLock::new(|| Regex::new(r"\b(?:pub\s+)?(?:async\s+)?(?:unsafe\s+)?fn\s+(\w+)").unwrap());
static RE_DEF_RUST_TYPE: LazyLock<Regex> =
LazyLock::new(|| Regex::new(r"\b(?:pub\s+)?(?:struct|enum|trait|type)\s+(\w+)").unwrap());
static RE_DEF_PY_FN: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"\bdef\s+(\w+)").unwrap());
static RE_DEF_PY_TYPE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"\bclass\s+(\w+)").unwrap());
static RE_DEF_JS_FN: LazyLock<Regex> =
LazyLock::new(|| Regex::new(r"\bfunction\s+\*?\s*(\w+)").unwrap());
static RE_DEF_JS_TYPE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"\bclass\s+(\w+)").unwrap());
static RE_DEF_GO_FN: LazyLock<Regex> =
LazyLock::new(|| Regex::new(r"\bfunc\s+(?:\([^)]*\)\s*)?(\w+)\s*\(").unwrap());
static RE_DEF_GO_TYPE: LazyLock<Regex> =
LazyLock::new(|| Regex::new(r"\btype\s+(\w+)\s+(?:struct|interface)").unwrap());
static RE_DEF_JAVA_TYPE: LazyLock<Regex> =
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<SymbolKind> {
let mut symbols = Vec::new();
Expand Down Expand Up @@ -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<crate::engine::context::types::DefinedSymbol> {
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::*;
Expand Down Expand Up @@ -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(
Expand Down
8 changes: 5 additions & 3 deletions src/engine/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
Loading
Loading