Skip to content

Commit b185b78

Browse files
committed
feat(review): annotate findings with blast radius
1 parent 5b4f5f9 commit b185b78

File tree

3 files changed

+206
-2
lines changed

3 files changed

+206
-2
lines changed

TODO.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ This roadmap is derived from deep research into Greptile's public docs, blog, MC
6565
32. [x] Add caller/callee expansion APIs for multi-hop impact analysis from changed symbols.
6666
33. [ ] Add contract edges between interfaces, implementations, and API endpoints.
6767
34. [ ] Add "similar implementation" lookup so repeated patterns and divergences are explicit.
68-
35. [ ] Add cross-file blast-radius summaries to findings when a change affects many callers.
68+
35. [x] Add cross-file blast-radius summaries to findings when a change affects many callers.
6969
36. [ ] Add graph freshness/version metadata so reviews know whether they are using stale repository intelligence.
7070
37. [x] Add graph-backed ranking of related files before semantic RAG retrieval.
7171
38. [x] Add graph query traces to `dag_traces` or review artifacts for explainability and debugging.
Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
use std::path::PathBuf;
2+
3+
use crate::core;
4+
5+
const MIN_BLAST_RADIUS_FILES: usize = 3;
6+
const MAX_LISTED_DEPENDENTS: usize = 3;
7+
const BLAST_RADIUS_TAG: &str = "blast-radius";
8+
const BLAST_RADIUS_PREFIX: &str = "Blast radius:";
9+
10+
pub(super) fn apply_blast_radius_summaries(
11+
mut comments: Vec<core::Comment>,
12+
symbol_index: Option<&core::SymbolIndex>,
13+
) -> Vec<core::Comment> {
14+
let Some(symbol_index) = symbol_index else {
15+
return comments;
16+
};
17+
18+
for comment in &mut comments {
19+
let mut dependents = symbol_index.reverse_deps(&comment.file_path);
20+
dependents.sort();
21+
dependents.dedup();
22+
23+
if dependents.len() < MIN_BLAST_RADIUS_FILES {
24+
continue;
25+
}
26+
27+
if !comment.content.contains(BLAST_RADIUS_PREFIX) {
28+
comment.content.push_str("\n\n");
29+
comment
30+
.content
31+
.push_str(&format_blast_radius_summary(&dependents));
32+
}
33+
34+
push_unique_tag(&mut comment.tags, BLAST_RADIUS_TAG);
35+
push_unique_tag(
36+
&mut comment.tags,
37+
&format!("blast-radius:{}", dependents.len()),
38+
);
39+
}
40+
41+
comments
42+
}
43+
44+
fn format_blast_radius_summary(dependents: &[PathBuf]) -> String {
45+
let listed = dependents
46+
.iter()
47+
.take(MAX_LISTED_DEPENDENTS)
48+
.map(|path| path.display().to_string())
49+
.collect::<Vec<_>>();
50+
let remaining = dependents.len().saturating_sub(listed.len());
51+
52+
let mut summary = format!(
53+
"{BLAST_RADIUS_PREFIX} {} dependent files reference this file ({})",
54+
dependents.len(),
55+
listed.join(", ")
56+
);
57+
if remaining > 0 {
58+
summary.push_str(&format!(", +{remaining} more"));
59+
}
60+
summary.push('.');
61+
summary
62+
}
63+
64+
fn push_unique_tag(tags: &mut Vec<String>, tag: &str) {
65+
if !tags.iter().any(|existing| existing == tag) {
66+
tags.push(tag.to_string());
67+
}
68+
}
69+
70+
#[cfg(test)]
71+
mod tests {
72+
use super::apply_blast_radius_summaries;
73+
use crate::core;
74+
use crate::core::comment::{Category, CommentStatus, FixEffort, Severity};
75+
use std::fs;
76+
use std::path::{Path, PathBuf};
77+
78+
fn build_index(repo_root: &Path) -> core::SymbolIndex {
79+
core::SymbolIndex::build(repo_root, 32, 128 * 1024, 8, |_path| false).unwrap()
80+
}
81+
82+
fn write_repo_file(repo_root: &Path, relative: &str, content: &str) {
83+
let path = repo_root.join(relative);
84+
if let Some(parent) = path.parent() {
85+
fs::create_dir_all(parent).unwrap();
86+
}
87+
fs::write(path, content).unwrap();
88+
}
89+
90+
fn make_comment(file_path: &str) -> core::Comment {
91+
core::Comment {
92+
id: "comment-1".to_string(),
93+
file_path: PathBuf::from(file_path),
94+
line_number: 1,
95+
content: "Shared helper has a correctness bug.".to_string(),
96+
rule_id: None,
97+
severity: Severity::Warning,
98+
category: Category::Bug,
99+
suggestion: None,
100+
confidence: 0.9,
101+
code_suggestion: None,
102+
tags: Vec::new(),
103+
fix_effort: FixEffort::Medium,
104+
feedback: None,
105+
status: CommentStatus::Open,
106+
resolved_at: None,
107+
}
108+
}
109+
110+
#[test]
111+
fn applies_blast_radius_summary_for_many_dependents() {
112+
let dir = tempfile::tempdir().unwrap();
113+
write_repo_file(dir.path(), "src/shared.rs", "pub fn helper() {}\n");
114+
write_repo_file(
115+
dir.path(),
116+
"src/a.rs",
117+
"const SHARED: &str = include_str!(\"./shared.rs\");\n",
118+
);
119+
write_repo_file(
120+
dir.path(),
121+
"src/b.rs",
122+
"const SHARED: &str = include_str!(\"./shared.rs\");\n",
123+
);
124+
write_repo_file(
125+
dir.path(),
126+
"src/c.rs",
127+
"const SHARED: &str = include_str!(\"./shared.rs\");\n",
128+
);
129+
130+
let index = build_index(dir.path());
131+
let comments =
132+
apply_blast_radius_summaries(vec![make_comment("src/shared.rs")], Some(&index));
133+
134+
assert_eq!(comments.len(), 1);
135+
assert!(comments[0]
136+
.content
137+
.contains("Blast radius: 3 dependent files reference this file"));
138+
assert!(comments[0].content.contains("src/a.rs"));
139+
assert!(comments[0].tags.iter().any(|tag| tag == "blast-radius"));
140+
assert!(comments[0].tags.iter().any(|tag| tag == "blast-radius:3"));
141+
}
142+
143+
#[test]
144+
fn skips_blast_radius_summary_below_threshold() {
145+
let dir = tempfile::tempdir().unwrap();
146+
write_repo_file(dir.path(), "src/shared.rs", "pub fn helper() {}\n");
147+
write_repo_file(
148+
dir.path(),
149+
"src/a.rs",
150+
"const SHARED: &str = include_str!(\"./shared.rs\");\n",
151+
);
152+
write_repo_file(
153+
dir.path(),
154+
"src/b.rs",
155+
"const SHARED: &str = include_str!(\"./shared.rs\");\n",
156+
);
157+
158+
let index = build_index(dir.path());
159+
let comments =
160+
apply_blast_radius_summaries(vec![make_comment("src/shared.rs")], Some(&index));
161+
162+
assert_eq!(comments[0].content, "Shared helper has a correctness bug.");
163+
assert!(comments[0].tags.is_empty());
164+
}
165+
}

src/review/pipeline/postprocess/dag.rs

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ use anyhow::Result;
22
use futures::FutureExt;
33
use tracing::{debug, info};
44

5+
#[path = "blast_radius.rs"]
6+
mod blast_radius;
7+
58
use crate::core;
69
use crate::core::dag::{
710
describe_dag, execute_dag, DagExecutionTrace, DagGraphContract, DagNode, DagNodeContract,
@@ -18,6 +21,7 @@ use super::dedup::deduplicate_specialized_comments;
1821
use super::feedback::apply_semantic_feedback_adjustment;
1922
use super::suppression::apply_convention_suppression;
2023
use super::verification::{apply_verification_pass, VerificationPassOutput};
24+
use blast_radius::apply_blast_radius_summaries;
2125

2226
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
2327
enum ReviewPostprocessStage {
@@ -29,6 +33,7 @@ enum ReviewPostprocessStage {
2933
ReviewFilters,
3034
EnhancedFilters,
3135
ConventionSuppression,
36+
BlastRadius,
3237
PrioritizeFindings,
3338
SaveConventionStore,
3439
}
@@ -44,6 +49,7 @@ impl DagNode for ReviewPostprocessStage {
4449
Self::ReviewFilters => "review_filters",
4550
Self::EnhancedFilters => "enhanced_filters",
4651
Self::ConventionSuppression => "convention_suppression",
52+
Self::BlastRadius => "blast_radius",
4753
Self::PrioritizeFindings => "prioritize_findings",
4854
Self::SaveConventionStore => "save_convention_store",
4955
}
@@ -298,6 +304,22 @@ pub(in super::super) fn describe_review_postprocess_graph(
298304
hints: stage_hints(spec.id),
299305
enabled: spec.enabled,
300306
},
307+
ReviewPostprocessStage::BlastRadius => DagNodeContract {
308+
name: spec.id.name().to_string(),
309+
description:
310+
"Annotate findings with cross-file blast-radius summaries from reverse dependency counts."
311+
.to_string(),
312+
kind: DagNodeKind::Transformation,
313+
dependencies: spec
314+
.dependencies
315+
.into_iter()
316+
.map(|dependency| dependency.name().to_string())
317+
.collect(),
318+
inputs: vec!["comments".to_string(), "symbol_index".to_string()],
319+
outputs: vec!["comments".to_string()],
320+
hints: stage_hints(spec.id),
321+
enabled: spec.enabled,
322+
},
301323
ReviewPostprocessStage::PrioritizeFindings => DagNodeContract {
302324
name: spec.id.name().to_string(),
303325
description:
@@ -405,8 +427,14 @@ fn build_postprocess_specs(
405427
enabled: true,
406428
},
407429
DagNodeSpec {
408-
id: ReviewPostprocessStage::PrioritizeFindings,
430+
id: ReviewPostprocessStage::BlastRadius,
409431
dependencies: vec![ReviewPostprocessStage::ConventionSuppression],
432+
hints: stage_hints(ReviewPostprocessStage::BlastRadius),
433+
enabled: true,
434+
},
435+
DagNodeSpec {
436+
id: ReviewPostprocessStage::PrioritizeFindings,
437+
dependencies: vec![ReviewPostprocessStage::BlastRadius],
410438
hints: stage_hints(ReviewPostprocessStage::PrioritizeFindings),
411439
enabled: true,
412440
},
@@ -436,6 +464,7 @@ async fn execute_stage(
436464
ReviewPostprocessStage::ConventionSuppression => {
437465
execute_convention_suppression_stage(context)
438466
}
467+
ReviewPostprocessStage::BlastRadius => execute_blast_radius_stage(context),
439468
ReviewPostprocessStage::PrioritizeFindings => execute_prioritize_findings_stage(context),
440469
ReviewPostprocessStage::SaveConventionStore => execute_convention_store_save_stage(context),
441470
}
@@ -529,6 +558,13 @@ fn execute_convention_suppression_stage(
529558
Ok(())
530559
}
531560

561+
fn execute_blast_radius_stage(context: &mut ReviewPostprocessDagContext<'_>) -> Result<()> {
562+
let comments = std::mem::take(&mut context.comments);
563+
context.comments =
564+
apply_blast_radius_summaries(comments, context.session.symbol_index.as_ref());
565+
Ok(())
566+
}
567+
532568
fn execute_prioritize_findings_stage(context: &mut ReviewPostprocessDagContext<'_>) -> Result<()> {
533569
core::sort_comments_by_priority(&mut context.comments);
534570
Ok(())
@@ -563,6 +599,9 @@ mod tests {
563599
.unwrap(),
564600
vec!["prioritize_findings"]
565601
);
602+
assert!(descriptions
603+
.iter()
604+
.any(|description| description.name == "blast_radius"));
566605
assert!(descriptions
567606
.iter()
568607
.any(|description| description.name == "prioritize_findings"));

0 commit comments

Comments
 (0)