Skip to content

Commit a72d399

Browse files
committed
feat(review): tag findings with context sources
1 parent cc4815a commit a72d399

File tree

6 files changed

+343
-4
lines changed

6 files changed

+343
-4
lines changed

TODO.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ This roadmap is derived from deep research into Greptile's public docs, blog, MC
8181
45. [ ] Support Jira/Linear issue context ingestion for PR-linked reviews.
8282
46. [ ] Support document-backed context ingestion for design docs, RFCs, and runbooks.
8383
47. [ ] Add explicit "intent mismatch" review checks comparing PR changes to ticket acceptance criteria.
84-
48. [ ] Add review artifacts that show which external context sources influenced a finding.
84+
48. [x] Add review artifacts that show which external context sources influenced a finding.
8585
49. [x] Add tests for pattern repository resolution across local paths, Git URLs, and broken sources.
8686
50. [ ] Add analytics on which context sources actually improve acceptance and fix rates.
8787

src/core/context_provenance.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,27 @@ impl ContextProvenance {
117117
}
118118
}
119119

120+
pub fn artifact_tag(&self) -> Option<String> {
121+
let source = match self {
122+
Self::ActiveReviewRules | Self::Analyzer { .. } => return None,
123+
Self::CustomContextNotes => "custom-context".to_string(),
124+
Self::DependencyGraphNeighborhood => "dependency-graph".to_string(),
125+
Self::PathSpecificFocusAreas => "path-focus".to_string(),
126+
Self::RepositoryGraphMetadata => "repository-graph".to_string(),
127+
Self::PatternRepositoryContext { source }
128+
| Self::PatternRepositorySource { source } => {
129+
format!("pattern-repository:{}", source.trim())
130+
}
131+
Self::RelatedTestFile => "related-test-file".to_string(),
132+
Self::ReverseDependencySummary => "reverse-dependency-summary".to_string(),
133+
Self::SimilarImplementation { .. } => "similar-implementation".to_string(),
134+
Self::SemanticRetrieval { .. } => "semantic-retrieval".to_string(),
135+
Self::SymbolGraphPath { .. } => "symbol-graph".to_string(),
136+
};
137+
138+
Some(format!("context-source:{source}"))
139+
}
140+
120141
fn label(&self) -> String {
121142
match self {
122143
Self::ActiveReviewRules => "active review rules".to_string(),
@@ -214,4 +235,29 @@ mod tests {
214235
"repository graph metadata"
215236
);
216237
}
238+
239+
#[test]
240+
fn artifact_tags_are_stable_for_external_context_sources() {
241+
assert_eq!(
242+
ContextProvenance::CustomContextNotes
243+
.artifact_tag()
244+
.as_deref(),
245+
Some("context-source:custom-context")
246+
);
247+
assert_eq!(
248+
ContextProvenance::pattern_repository_source("Acme/security-rules")
249+
.artifact_tag()
250+
.as_deref(),
251+
Some("context-source:pattern-repository:Acme/security-rules")
252+
);
253+
assert_eq!(
254+
ContextProvenance::symbol_graph_path(vec!["calls".to_string()], 1, 0.7)
255+
.artifact_tag()
256+
.as_deref(),
257+
Some("context-source:symbol-graph")
258+
);
259+
assert!(ContextProvenance::ActiveReviewRules
260+
.artifact_tag()
261+
.is_none());
262+
}
217263
}

src/review/context_helpers/injection.rs

Lines changed: 122 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,12 @@ pub async fn inject_custom_context(
2424
}
2525

2626
if !entry.files.is_empty() {
27-
let extra_chunks = context_fetcher
27+
let mut extra_chunks = context_fetcher
2828
.fetch_additional_context(&entry.files)
2929
.await?;
30+
for chunk in &mut extra_chunks {
31+
chunk.provenance = Some(core::ContextProvenance::CustomContextNotes);
32+
}
3033
context_chunks.extend(extra_chunks);
3134
}
3235
}
@@ -85,3 +88,121 @@ pub async fn inject_pattern_repository_context(
8588

8689
Ok(())
8790
}
91+
92+
#[cfg(test)]
93+
mod tests {
94+
use std::collections::HashMap;
95+
use std::fs;
96+
use std::path::PathBuf;
97+
98+
use super::*;
99+
100+
fn diff_for(path: &str) -> core::UnifiedDiff {
101+
core::UnifiedDiff {
102+
file_path: PathBuf::from(path),
103+
old_content: None,
104+
new_content: None,
105+
hunks: Vec::new(),
106+
is_binary: false,
107+
is_deleted: false,
108+
is_new: false,
109+
}
110+
}
111+
112+
#[tokio::test]
113+
async fn inject_custom_context_tags_repo_files_with_custom_context_provenance() {
114+
let dir = tempfile::tempdir().unwrap();
115+
fs::create_dir_all(dir.path().join("docs")).unwrap();
116+
fs::write(
117+
dir.path().join("docs/review-notes.md"),
118+
"Prefer contract-safe status names.",
119+
)
120+
.unwrap();
121+
122+
let mut config = config::Config::default();
123+
config.custom_context = vec![config::CustomContextConfig {
124+
scope: Some("src/**/*.rs".to_string()),
125+
notes: Vec::new(),
126+
files: vec!["docs/review-notes.md".to_string()],
127+
}];
128+
129+
let fetcher = core::ContextFetcher::new(dir.path().to_path_buf());
130+
let mut context_chunks = Vec::new();
131+
132+
inject_custom_context(
133+
&config,
134+
&fetcher,
135+
&diff_for("src/lib.rs"),
136+
&mut context_chunks,
137+
)
138+
.await
139+
.unwrap();
140+
141+
assert_eq!(context_chunks.len(), 1);
142+
assert_eq!(
143+
context_chunks[0].provenance,
144+
Some(core::ContextProvenance::CustomContextNotes)
145+
);
146+
assert_eq!(
147+
context_chunks[0].file_path,
148+
PathBuf::from("docs/review-notes.md")
149+
);
150+
}
151+
152+
#[tokio::test]
153+
async fn inject_pattern_repository_context_tags_source_and_chunks() {
154+
let dir = tempfile::tempdir().unwrap();
155+
let repo_root = dir.path();
156+
let pattern_repo = repo_root.join("patterns/security-rules");
157+
fs::create_dir_all(pattern_repo.join("rules")).unwrap();
158+
fs::write(
159+
pattern_repo.join("rules/sql.md"),
160+
"Always parameterize SQL queries.",
161+
)
162+
.unwrap();
163+
164+
let mut config = config::Config::default();
165+
config.pattern_repositories = vec![config::PatternRepositoryConfig {
166+
source: "patterns/security-rules".to_string(),
167+
include_patterns: vec!["rules/*.md".to_string()],
168+
max_files: 5,
169+
max_lines: 20,
170+
..Default::default()
171+
}];
172+
173+
let fetcher = core::ContextFetcher::new(repo_root.to_path_buf());
174+
let mut resolved = HashMap::new();
175+
resolved.insert(
176+
"patterns/security-rules".to_string(),
177+
pattern_repo.canonicalize().unwrap(),
178+
);
179+
let mut context_chunks = Vec::new();
180+
181+
inject_pattern_repository_context(
182+
&config,
183+
&resolved,
184+
&fetcher,
185+
&diff_for("src/lib.rs"),
186+
&mut context_chunks,
187+
)
188+
.await
189+
.unwrap();
190+
191+
assert_eq!(context_chunks.len(), 2);
192+
assert_eq!(
193+
context_chunks[0].provenance,
194+
Some(core::ContextProvenance::pattern_repository_source(
195+
"patterns/security-rules"
196+
))
197+
);
198+
assert_eq!(
199+
context_chunks[1].provenance,
200+
Some(core::ContextProvenance::pattern_repository_context(
201+
"patterns/security-rules"
202+
))
203+
);
204+
assert!(context_chunks[1]
205+
.content
206+
.starts_with("[Pattern repository: patterns/security-rules]"));
207+
}
208+
}

src/review/pipeline/file_context/sources/supplemental.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,10 +176,13 @@ pub(in super::super) async fn add_path_context(
176176
}
177177

178178
if !path_config.extra_context.is_empty() {
179-
let extra_chunks = services
179+
let mut extra_chunks = services
180180
.context_fetcher
181181
.fetch_additional_context(&path_config.extra_context)
182182
.await?;
183+
for chunk in &mut extra_chunks {
184+
chunk.provenance = Some(core::ContextProvenance::PathSpecificFocusAreas);
185+
}
183186
context_chunks.extend(extra_chunks);
184187
}
185188

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
use std::collections::{BTreeSet, HashMap};
2+
use std::path::PathBuf;
3+
4+
use crate::core;
5+
6+
pub(super) fn apply_context_source_tags(
7+
mut comments: Vec<core::Comment>,
8+
verification_context: &HashMap<PathBuf, Vec<core::LLMContextChunk>>,
9+
) -> Vec<core::Comment> {
10+
let tags_by_file = verification_context
11+
.iter()
12+
.filter_map(|(file_path, chunks)| {
13+
let tags = artifact_tags_for_chunks(chunks);
14+
(!tags.is_empty()).then(|| (file_path.clone(), tags))
15+
})
16+
.collect::<HashMap<_, _>>();
17+
18+
for comment in &mut comments {
19+
let Some(tags) = tags_by_file.get(&comment.file_path) else {
20+
continue;
21+
};
22+
23+
for tag in tags {
24+
push_unique_tag(&mut comment.tags, tag);
25+
}
26+
}
27+
28+
comments
29+
}
30+
31+
fn artifact_tags_for_chunks(chunks: &[core::LLMContextChunk]) -> Vec<String> {
32+
chunks
33+
.iter()
34+
.filter_map(|chunk| chunk.provenance.as_ref())
35+
.filter_map(core::ContextProvenance::artifact_tag)
36+
.collect::<BTreeSet<_>>()
37+
.into_iter()
38+
.collect()
39+
}
40+
41+
fn push_unique_tag(tags: &mut Vec<String>, tag: &str) {
42+
if !tags.iter().any(|existing| existing == tag) {
43+
tags.push(tag.to_string());
44+
}
45+
}
46+
47+
#[cfg(test)]
48+
mod tests {
49+
use std::collections::HashMap;
50+
use std::path::PathBuf;
51+
52+
use super::*;
53+
use crate::core::comment::{Category, CommentStatus, FixEffort, Severity};
54+
55+
fn make_comment(file_path: &str) -> core::Comment {
56+
core::Comment {
57+
id: "comment-1".to_string(),
58+
file_path: PathBuf::from(file_path),
59+
line_number: 7,
60+
content: "Missing validation.".to_string(),
61+
rule_id: None,
62+
severity: Severity::Warning,
63+
category: Category::Bug,
64+
suggestion: None,
65+
confidence: 0.8,
66+
code_suggestion: None,
67+
tags: vec!["security".to_string()],
68+
fix_effort: FixEffort::Medium,
69+
feedback: None,
70+
status: CommentStatus::Open,
71+
resolved_at: None,
72+
}
73+
}
74+
75+
#[test]
76+
fn applies_unique_context_source_tags_for_matching_file() {
77+
let mut verification_context = HashMap::new();
78+
verification_context.insert(
79+
PathBuf::from("src/api.rs"),
80+
vec![
81+
core::LLMContextChunk::documentation(
82+
PathBuf::from("src/api.rs"),
83+
"Custom context notes".to_string(),
84+
)
85+
.with_provenance(core::ContextProvenance::CustomContextNotes),
86+
core::LLMContextChunk::reference(
87+
PathBuf::from("patterns/sql.md"),
88+
"Pattern repository rule".to_string(),
89+
)
90+
.with_provenance(
91+
core::ContextProvenance::pattern_repository_context("patterns/security-rules"),
92+
),
93+
core::LLMContextChunk::reference(
94+
PathBuf::from("patterns/sql.md"),
95+
"Pattern repository rule".to_string(),
96+
)
97+
.with_provenance(
98+
core::ContextProvenance::pattern_repository_source("patterns/security-rules"),
99+
),
100+
core::LLMContextChunk::documentation(
101+
PathBuf::from("src/api.rs"),
102+
"Analyzer guidance".to_string(),
103+
)
104+
.with_provenance(core::ContextProvenance::analyzer("contracts")),
105+
],
106+
);
107+
108+
let comments = apply_context_source_tags(
109+
vec![make_comment("src/api.rs"), make_comment("src/other.rs")],
110+
&verification_context,
111+
);
112+
113+
assert_eq!(
114+
comments[0].tags,
115+
vec![
116+
"security".to_string(),
117+
"context-source:custom-context".to_string(),
118+
"context-source:pattern-repository:patterns/security-rules".to_string(),
119+
]
120+
);
121+
assert_eq!(comments[1].tags, vec!["security".to_string()]);
122+
}
123+
}

0 commit comments

Comments
 (0)