Skip to content

Commit 5b4f5f9

Browse files
committed
feat(review): trace graph context queries
Capture symbol-graph, reverse-dependency, and graph-ranked semantic lookups as graph_query dag traces during file-context assembly, carry them through review results and eval artifacts, add focused trace tests, and mark TODO.md item #38 complete.
1 parent 4ed9723 commit 5b4f5f9

File tree

12 files changed

+289
-13
lines changed

12 files changed

+289
-13
lines changed

TODO.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ This roadmap is derived from deep research into Greptile's public docs, blog, MC
6868
35. [ ] 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.
71-
38. [ ] Add graph query traces to `dag_traces` or review artifacts for explainability and debugging.
71+
38. [x] Add graph query traces to `dag_traces` or review artifacts for explainability and debugging.
7272
39. [x] Add graph-aware eval fixtures that require multi-hop code understanding to pass.
7373
40. [ ] Split `src/core/symbol_graph.rs` into construction, persistence, traversal, and ranking modules as it grows.
7474

src/review/pipeline/file_context.rs

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ mod sources;
1010
use crate::config;
1111
use crate::core;
1212

13+
use super::context::extract_symbols_from_diff;
1314
use super::services::PipelineServices;
1415
use super::session::ReviewSession;
1516

@@ -18,6 +19,7 @@ pub(super) struct PreparedFileContext {
1819
pub path_config: Option<config::PathConfig>,
1920
pub deterministic_comments: Vec<core::Comment>,
2021
pub context_chunks: Vec<core::LLMContextChunk>,
22+
pub graph_query_traces: Vec<core::dag::DagExecutionTrace>,
2123
}
2224

2325
pub(super) async fn assemble_file_context(
@@ -30,18 +32,66 @@ pub(super) async fn assemble_file_context(
3032
let path_config = services.config.get_path_config(&diff.file_path).cloned();
3133
let mut context_chunks =
3234
base::initial_context_chunks(services, diff, pre_analysis_context).await?;
35+
let diff_symbols = extract_symbols_from_diff(diff);
36+
let mut graph_query_records = Vec::new();
3337

34-
sources::add_symbol_context(services, session, diff, &mut context_chunks).await?;
35-
sources::add_related_file_context(services, session, diff, &mut context_chunks);
36-
sources::add_semantic_context(services, session, diff, &mut context_chunks).await;
38+
sources::add_symbol_context(
39+
services,
40+
session,
41+
diff,
42+
&diff_symbols,
43+
&mut context_chunks,
44+
&mut graph_query_records,
45+
)
46+
.await?;
47+
sources::add_related_file_context(
48+
services,
49+
session,
50+
diff,
51+
&mut context_chunks,
52+
&mut graph_query_records,
53+
);
54+
sources::add_semantic_context(
55+
services,
56+
session,
57+
diff,
58+
&diff_symbols,
59+
&mut context_chunks,
60+
&mut graph_query_records,
61+
)
62+
.await;
3763
sources::add_path_context(services, diff, path_config.as_ref(), &mut context_chunks).await?;
3864
sources::inject_repository_context(services, diff, &mut context_chunks).await?;
3965

66+
if !diff_symbols.is_empty() && !graph_query_records.is_empty() {
67+
graph_query_records.insert(
68+
0,
69+
sources::trace_record(
70+
format!("seed_symbols={}", summarize_seed_symbols(&diff_symbols)),
71+
0,
72+
),
73+
);
74+
}
75+
76+
let graph_query_traces = sources::build_graph_query_trace(&diff.file_path, graph_query_records)
77+
.into_iter()
78+
.collect();
79+
4080
Ok(finalize::finalize_file_context(
4181
services,
4282
diff,
4383
path_config,
4484
deterministic_comments,
4585
context_chunks,
86+
graph_query_traces,
4687
))
4788
}
89+
90+
fn summarize_seed_symbols(symbols: &[String]) -> String {
91+
let shown = symbols.iter().take(6).cloned().collect::<Vec<_>>();
92+
let mut summary = shown.join(", ");
93+
if symbols.len() > shown.len() {
94+
summary.push_str(&format!(" (+{} more)", symbols.len() - shown.len()));
95+
}
96+
summary
97+
}

src/review/pipeline/file_context/finalize.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ pub(super) fn finalize_file_context(
1010
path_config: Option<config::PathConfig>,
1111
deterministic_comments: Vec<core::Comment>,
1212
mut context_chunks: Vec<core::LLMContextChunk>,
13+
graph_query_traces: Vec<core::dag::DagExecutionTrace>,
1314
) -> PreparedFileContext {
1415
let active_rules = core::active_rules_for_file(
1516
&services.review_rules,
@@ -33,5 +34,6 @@ pub(super) fn finalize_file_context(
3334
path_config,
3435
deterministic_comments,
3536
context_chunks,
37+
graph_query_traces,
3638
}
3739
}

src/review/pipeline/file_context/sources.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,11 @@ mod repo;
66
mod supplemental;
77
#[path = "sources/symbols.rs"]
88
mod symbols;
9+
#[path = "sources/traces.rs"]
10+
mod traces;
911

1012
pub(super) use related::add_related_file_context;
1113
pub(super) use repo::inject_repository_context;
1214
pub(super) use supplemental::{add_path_context, add_semantic_context};
1315
pub(super) use symbols::add_symbol_context;
16+
pub(super) use traces::{build_graph_query_trace, trace_record};
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,85 @@
1+
use std::time::Instant;
2+
13
use crate::core;
24

35
use super::super::super::context::gather_related_file_context;
46
use super::super::super::services::PipelineServices;
57
use super::super::super::session::ReviewSession;
8+
use super::traces::{trace_record, MAX_GRAPH_TRACE_DETAILS};
69

710
pub(in super::super) fn add_related_file_context(
811
services: &PipelineServices,
912
session: &ReviewSession,
1013
diff: &core::UnifiedDiff,
1114
context_chunks: &mut Vec<core::LLMContextChunk>,
15+
graph_query_records: &mut Vec<core::dag::DagExecutionRecord>,
1216
) {
1317
if let Some(index) = session.symbol_index.as_ref() {
18+
let lookup_started = Instant::now();
1419
let caller_chunks =
1520
gather_related_file_context(index, &diff.file_path, &services.repo_path);
21+
append_related_file_trace_records(
22+
graph_query_records,
23+
&caller_chunks,
24+
lookup_started.elapsed().as_millis() as u64,
25+
);
1626
context_chunks.extend(caller_chunks);
1727
}
1828
}
29+
30+
fn append_related_file_trace_records(
31+
graph_query_records: &mut Vec<core::dag::DagExecutionRecord>,
32+
caller_chunks: &[core::LLMContextChunk],
33+
duration_ms: u64,
34+
) {
35+
let reverse_dependencies = caller_chunks
36+
.iter()
37+
.filter(|chunk| {
38+
matches!(
39+
chunk.provenance.as_ref(),
40+
Some(core::ContextProvenance::ReverseDependencySummary)
41+
)
42+
})
43+
.map(|chunk| chunk.file_path.display().to_string())
44+
.collect::<Vec<_>>();
45+
46+
graph_query_records.push(trace_record(
47+
format!("reverse_dependency_hits={}", reverse_dependencies.len()),
48+
duration_ms,
49+
));
50+
for (index, file_path) in reverse_dependencies
51+
.iter()
52+
.take(MAX_GRAPH_TRACE_DETAILS)
53+
.enumerate()
54+
{
55+
graph_query_records.push(trace_record(
56+
format!("reverse_dependency_hit[{index}]={file_path}"),
57+
0,
58+
));
59+
}
60+
}
61+
62+
#[cfg(test)]
63+
mod tests {
64+
use super::append_related_file_trace_records;
65+
use crate::core;
66+
use std::path::PathBuf;
67+
68+
#[test]
69+
fn related_file_trace_records_only_capture_reverse_dependencies() {
70+
let chunks = vec![
71+
core::LLMContextChunk::reference(PathBuf::from("src/caller.rs"), "caller".to_string())
72+
.with_provenance(core::ContextProvenance::ReverseDependencySummary),
73+
core::LLMContextChunk::reference(PathBuf::from("tests/file.rs"), "test".to_string())
74+
.with_provenance(core::ContextProvenance::RelatedTestFile),
75+
];
76+
77+
let mut records = Vec::new();
78+
append_related_file_trace_records(&mut records, &chunks, 4);
79+
80+
assert_eq!(records[0].name, "reverse_dependency_hits=1");
81+
assert_eq!(records[0].duration_ms, 4);
82+
assert_eq!(records[1].name, "reverse_dependency_hit[0]=src/caller.rs");
83+
assert_eq!(records.len(), 2);
84+
}
85+
}

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

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,37 @@
11
use std::collections::HashSet;
22
use std::path::PathBuf;
3+
use std::time::Instant;
34

45
use anyhow::Result;
56

67
use crate::config;
78
use crate::core;
89

9-
use super::super::super::context::extract_symbols_from_diff;
1010
use super::super::super::services::PipelineServices;
1111
use super::super::super::session::ReviewSession;
12+
use super::traces::{trace_record, MAX_GRAPH_TRACE_DETAILS};
1213

1314
pub(in super::super) async fn add_semantic_context(
1415
services: &PipelineServices,
1516
session: &ReviewSession,
1617
diff: &core::UnifiedDiff,
18+
symbols: &[String],
1719
context_chunks: &mut Vec<core::LLMContextChunk>,
20+
graph_query_records: &mut Vec<core::dag::DagExecutionRecord>,
1821
) {
1922
let Some(index) = session.semantic_index.as_ref() else {
2023
return;
2124
};
22-
let preferred_files = graph_ranked_semantic_files(services, session, diff);
25+
let should_trace_graph_ranking = session.symbol_index.is_some() && !symbols.is_empty();
26+
let ranking_started = Instant::now();
27+
let preferred_files = graph_ranked_semantic_files(services, session, diff, symbols);
28+
if should_trace_graph_ranking {
29+
append_semantic_preference_trace_records(
30+
graph_query_records,
31+
&preferred_files,
32+
ranking_started.elapsed().as_millis() as u64,
33+
);
34+
}
2335

2436
let semantic_chunks = core::semantic_context_for_diff(
2537
index,
@@ -41,12 +53,12 @@ fn graph_ranked_semantic_files(
4153
services: &PipelineServices,
4254
session: &ReviewSession,
4355
diff: &core::UnifiedDiff,
56+
symbols: &[String],
4457
) -> Vec<PathBuf> {
4558
let Some(index) = session.symbol_index.as_ref() else {
4659
return Vec::new();
4760
};
4861

49-
let symbols = extract_symbols_from_diff(diff);
5062
if symbols.is_empty() {
5163
return Vec::new();
5264
}
@@ -59,7 +71,7 @@ fn graph_ranked_semantic_files(
5971
services.config.symbol_index_graph_max_files,
6072
),
6173
);
62-
let related_locations = retriever.related_symbol_locations(&diff.file_path, &symbols);
74+
let related_locations = retriever.related_symbol_locations(&diff.file_path, symbols);
6375

6476
let mut preferred_files = Vec::new();
6577
let mut seen = HashSet::new();
@@ -84,6 +96,27 @@ fn graph_ranked_semantic_files(
8496
preferred_files
8597
}
8698

99+
fn append_semantic_preference_trace_records(
100+
graph_query_records: &mut Vec<core::dag::DagExecutionRecord>,
101+
preferred_files: &[PathBuf],
102+
duration_ms: u64,
103+
) {
104+
graph_query_records.push(trace_record(
105+
format!("semantic_preferred_files={}", preferred_files.len()),
106+
duration_ms,
107+
));
108+
for (index, file_path) in preferred_files
109+
.iter()
110+
.take(MAX_GRAPH_TRACE_DETAILS)
111+
.enumerate()
112+
{
113+
graph_query_records.push(trace_record(
114+
format!("semantic_preferred_file[{index}]={}", file_path.display()),
115+
0,
116+
));
117+
}
118+
}
119+
87120
pub(in super::super) async fn add_path_context(
88121
services: &PipelineServices,
89122
diff: &core::UnifiedDiff,
@@ -117,3 +150,22 @@ pub(in super::super) async fn add_path_context(
117150

118151
Ok(())
119152
}
153+
154+
#[cfg(test)]
155+
mod tests {
156+
use super::append_semantic_preference_trace_records;
157+
use std::path::PathBuf;
158+
159+
#[test]
160+
fn semantic_preference_trace_records_keep_ranked_order() {
161+
let preferred_files = vec![PathBuf::from("src/auth.rs"), PathBuf::from("src/db.rs")];
162+
163+
let mut records = Vec::new();
164+
append_semantic_preference_trace_records(&mut records, &preferred_files, 7);
165+
166+
assert_eq!(records[0].name, "semantic_preferred_files=2");
167+
assert_eq!(records[0].duration_ms, 7);
168+
assert_eq!(records[1].name, "semantic_preferred_file[0]=src/auth.rs");
169+
assert_eq!(records[2].name, "semantic_preferred_file[1]=src/db.rs");
170+
}
171+
}

0 commit comments

Comments
 (0)