Skip to content

Commit ea4c7ab

Browse files
committed
feat(review): cache verifier decisions across PR reruns
1 parent c0c0139 commit ea4c7ab

23 files changed

Lines changed: 631 additions & 90 deletions

File tree

TODO.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ This roadmap is derived from deep research into Greptile's public docs, blog, MC
5555
25. [x] Add a handoff contract from reviewer findings to fix agents with rule IDs, evidence, and suggested diffs.
5656
26. [x] Persist loop-level telemetry: iterations, fixes attempted, findings cleared, findings reopened.
5757
27. [x] Add "challenge the finding" verification loops where a validator tries to falsify a suspected issue before keeping it.
58-
28. [ ] Add caching between iterations so repeated codebase retrieval and verification runs are cheaper.
58+
28. [x] Add caching between iterations so repeated codebase retrieval and verification runs are cheaper.
5959
29. [x] Allow loop policies to differ by profile: conservative auditor, high-autonomy fixer, or report-only.
6060
30. [x] Add eval fixtures specifically for loop convergence and reopened-issue regressions.
6161

src/review/mod.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ pub use filters::apply_review_filters;
2323
pub use pipeline::{
2424
build_review_guidance, build_symbol_index, extract_symbols_from_diff, filter_comments_for_diff,
2525
review_diff_content, review_diff_content_raw, review_diff_content_raw_with_progress,
26-
review_diff_content_with_repo, ProgressCallback, ProgressUpdate,
26+
review_diff_content_raw_with_progress_and_verification_reuse,
27+
review_diff_content_raw_with_verification_reuse, review_diff_content_with_repo,
28+
ProgressCallback, ProgressUpdate,
2729
};
2830
#[allow(unused_imports)]
2931
pub(crate) use pipeline::{describe_review_pipeline_graph, describe_review_postprocess_graph};
@@ -36,6 +38,8 @@ pub use rule_helpers::{
3638
};
3739
#[allow(unused_imports)]
3840
pub(crate) use verification::summarize_review_verification;
41+
#[allow(unused_imports)]
42+
pub(crate) use verification::VerificationReuseCache;
3943

4044
// Used by sibling modules (commands, output) and their tests
4145
#[allow(unused_imports)]

src/review/pipeline.rs

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,23 @@ pub async fn review_diff_content_raw(
6969
config: config::Config,
7070
repo_path: &Path,
7171
) -> Result<ReviewResult> {
72-
review_diff_content_raw_with_progress(diff_content, config, repo_path, None).await
72+
review_diff_content_raw_internal(diff_content, config, repo_path, None, None).await
73+
}
74+
75+
pub async fn review_diff_content_raw_with_verification_reuse(
76+
diff_content: &str,
77+
config: config::Config,
78+
repo_path: &Path,
79+
verification_reuse_cache: crate::review::verification::VerificationReuseCache,
80+
) -> Result<ReviewResult> {
81+
review_diff_content_raw_internal(
82+
diff_content,
83+
config,
84+
repo_path,
85+
None,
86+
Some(verification_reuse_cache),
87+
)
88+
.await
7389
}
7490

7591
#[tracing::instrument(name = "review_pipeline", skip(diff_content, config, repo_path, on_progress), fields(diff_bytes = diff_content.len(), model = %config.model))]
@@ -79,16 +95,53 @@ pub async fn review_diff_content_raw_with_progress(
7995
repo_path: &Path,
8096
on_progress: Option<ProgressCallback>,
8197
) -> Result<ReviewResult> {
98+
review_diff_content_raw_internal(diff_content, config, repo_path, on_progress, None).await
99+
}
100+
101+
pub async fn review_diff_content_raw_with_progress_and_verification_reuse(
102+
diff_content: &str,
103+
config: config::Config,
104+
repo_path: &Path,
105+
on_progress: Option<ProgressCallback>,
106+
verification_reuse_cache: crate::review::verification::VerificationReuseCache,
107+
) -> Result<ReviewResult> {
108+
review_diff_content_raw_internal(
109+
diff_content,
110+
config,
111+
repo_path,
112+
on_progress,
113+
Some(verification_reuse_cache),
114+
)
115+
.await
116+
}
117+
118+
async fn review_diff_content_raw_internal(
119+
diff_content: &str,
120+
config: config::Config,
121+
repo_path: &Path,
122+
on_progress: Option<ProgressCallback>,
123+
verification_reuse_cache: Option<crate::review::verification::VerificationReuseCache>,
124+
) -> Result<ReviewResult> {
125+
let verification_reuse_cache = verification_reuse_cache.unwrap_or_default();
126+
82127
if let Some(result) = maybe_review_chunked_diff_content(
83128
diff_content,
84129
config.clone(),
85130
repo_path,
86131
on_progress.clone(),
132+
verification_reuse_cache.clone(),
87133
)
88134
.await?
89135
{
90136
return Ok(result);
91137
}
92138

93-
orchestrate::review_diff_content_raw_inner(diff_content, config, repo_path, on_progress).await
139+
orchestrate::review_diff_content_raw_inner(
140+
diff_content,
141+
config,
142+
repo_path,
143+
on_progress,
144+
verification_reuse_cache,
145+
)
146+
.await
94147
}

src/review/pipeline/chunking.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ pub(super) async fn maybe_review_chunked_diff_content(
1212
config: config::Config,
1313
repo_path: &Path,
1414
on_progress: Option<ProgressCallback>,
15+
verification_reuse_cache: crate::review::verification::VerificationReuseCache,
1516
) -> Result<Option<ReviewResult>> {
1617
if !should_optimize_for_local(&config) {
1718
return Ok(None);
@@ -30,17 +31,22 @@ pub(super) async fn maybe_review_chunked_diff_content(
3031
);
3132

3233
let mut merged = ReviewResult::default();
34+
let mut verification_reuse_cache = verification_reuse_cache;
3335
for (index, chunk) in chunks.iter().enumerate() {
3436
eprintln!("Processing chunk {}/{}...", index + 1, chunks.len());
3537
match super::orchestrate::review_diff_content_raw_inner(
3638
chunk,
3739
config.clone(),
3840
repo_path,
3941
on_progress.clone(),
42+
std::mem::take(&mut verification_reuse_cache),
4043
)
4144
.await
4245
{
43-
Ok(chunk_result) => merge_chunk_result(&mut merged, chunk_result),
46+
Ok(chunk_result) => {
47+
verification_reuse_cache = chunk_result.verification_reuse_cache.clone();
48+
merge_chunk_result(&mut merged, chunk_result)
49+
}
4450
Err(error) => {
4551
eprintln!("Warning: chunk {} failed: {}", index + 1, error);
4652
}
@@ -64,5 +70,8 @@ fn merge_chunk_result(merged: &mut ReviewResult, chunk_result: ReviewResult) {
6470
if merged.verification_report.is_none() {
6571
merged.verification_report = chunk_result.verification_report;
6672
}
73+
merged
74+
.verification_reuse_cache
75+
.merge(chunk_result.verification_reuse_cache);
6776
merged.warnings.extend(chunk_result.warnings);
6877
}

src/review/pipeline/orchestrate.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,14 @@ pub(super) async fn review_diff_content_raw_inner(
1818
config: config::Config,
1919
repo_path: &Path,
2020
on_progress: Option<ProgressCallback>,
21+
verification_reuse_cache: crate::review::verification::VerificationReuseCache,
2122
) -> Result<ReviewResult> {
22-
execute_review_pipeline_dag(diff_content, config, repo_path, on_progress).await
23+
execute_review_pipeline_dag(
24+
diff_content,
25+
config,
26+
repo_path,
27+
on_progress,
28+
verification_reuse_cache,
29+
)
30+
.await
2331
}

src/review/pipeline/orchestrate/dag.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ struct ReviewPipelineDagContext<'a> {
4343
config: Option<config::Config>,
4444
repo_path: &'a Path,
4545
on_progress: Option<ProgressCallback>,
46+
verification_reuse_cache: crate::review::verification::VerificationReuseCache,
4647
services: Option<PipelineServices>,
4748
session: Option<ReviewSession>,
4849
prepared_jobs: Option<PreparedReviewJobs>,
@@ -56,12 +57,14 @@ impl<'a> ReviewPipelineDagContext<'a> {
5657
config: config::Config,
5758
repo_path: &'a Path,
5859
on_progress: Option<ProgressCallback>,
60+
verification_reuse_cache: crate::review::verification::VerificationReuseCache,
5961
) -> Self {
6062
Self {
6163
diff_content,
6264
config: Some(config),
6365
repo_path,
6466
on_progress,
67+
verification_reuse_cache,
6568
services: None,
6669
session: None,
6770
prepared_jobs: None,
@@ -81,11 +84,18 @@ pub(super) async fn execute_review_pipeline_dag(
8184
config: config::Config,
8285
repo_path: &Path,
8386
on_progress: Option<ProgressCallback>,
87+
verification_reuse_cache: crate::review::verification::VerificationReuseCache,
8488
) -> Result<ReviewResult> {
8589
let specs = build_review_pipeline_specs();
8690
let dag_description = describe_dag(&specs);
8791
debug!(?dag_description, "Executing review pipeline DAG");
88-
let mut context = ReviewPipelineDagContext::new(diff_content, config, repo_path, on_progress);
92+
let mut context = ReviewPipelineDagContext::new(
93+
diff_content,
94+
config,
95+
repo_path,
96+
on_progress,
97+
verification_reuse_cache,
98+
);
8999
let records = execute_dag(&specs, &mut context, |stage, context| {
90100
async move { execute_stage(stage, context).await }.boxed()
91101
})
@@ -250,7 +260,13 @@ async fn execute_session_stage(context: &mut ReviewPipelineDagContext<'_>) -> Re
250260
.as_ref()
251261
.ok_or_else(|| anyhow::anyhow!("session stage missing services"))?;
252262
context.session = Some(
253-
ReviewSession::new(context.diff_content, services, context.on_progress.clone()).await?,
263+
ReviewSession::new(
264+
context.diff_content,
265+
services,
266+
context.on_progress.clone(),
267+
std::mem::take(&mut context.verification_reuse_cache),
268+
)
269+
.await?,
254270
);
255271
Ok(())
256272
}

src/review/pipeline/postprocess/dag.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ impl<'a> ReviewPostprocessDagContext<'a> {
108108
hotspots: self.session.enhanced_ctx.hotspots.clone(),
109109
agent_activity: self.agent_activity,
110110
verification_report,
111+
verification_reuse_cache: self.session.verification_reuse_cache.clone(),
111112
warnings,
112113
dag_traces: self.session.graph_query_traces.clone(),
113114
}

src/review/pipeline/postprocess/verification.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use tracing::{info, warn};
22

33
use crate::core;
4-
use crate::review::verification::VerificationJudgeConfig;
4+
use crate::review::verification::{verify_comments_with_judges_and_reuse, VerificationJudgeConfig};
55

66
use super::super::comments::is_analyzer_comment;
77
use super::super::services::PipelineServices;
@@ -16,7 +16,7 @@ pub(super) struct VerificationPassOutput {
1616
pub(super) async fn apply_verification_pass(
1717
comments: Vec<core::Comment>,
1818
services: &PipelineServices,
19-
session: &ReviewSession,
19+
session: &mut ReviewSession,
2020
) -> VerificationPassOutput {
2121
let (analyzer_comments, llm_comments): (Vec<_>, Vec<_>) =
2222
comments.into_iter().partition(is_analyzer_comment);
@@ -26,7 +26,7 @@ pub(super) async fn apply_verification_pass(
2626
&& llm_comments.len() <= services.config.verification.max_comments
2727
{
2828
let comment_count_before = llm_comments.len();
29-
let summary = super::super::super::verification::verify_comments_with_judges(
29+
let summary = verify_comments_with_judges_and_reuse(
3030
llm_comments,
3131
&session.diffs,
3232
&session.source_files,
@@ -37,6 +37,7 @@ pub(super) async fn apply_verification_pass(
3737
fail_open: services.config.verification.fail_open,
3838
consensus_mode: services.config.verification.consensus_mode,
3939
},
40+
Some(&mut session.verification_reuse_cache),
4041
)
4142
.await;
4243

src/review/pipeline/session.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ pub(super) struct ReviewSession {
2525
pub semantic_index: Option<core::semantic::SemanticIndex>,
2626
pub semantic_feedback_store: Option<core::SemanticFeedbackStore>,
2727
pub verification_context: HashMap<PathBuf, Vec<core::LLMContextChunk>>,
28+
pub verification_reuse_cache: crate::review::verification::VerificationReuseCache,
2829
pub graph_query_traces: Vec<core::dag::DagExecutionTrace>,
2930
}
3031

@@ -33,7 +34,14 @@ impl ReviewSession {
3334
diff_content: &str,
3435
services: &PipelineServices,
3536
on_progress: Option<ProgressCallback>,
37+
verification_reuse_cache: crate::review::verification::VerificationReuseCache,
3638
) -> Result<Self> {
37-
build_review_session(diff_content, services, on_progress).await
39+
build_review_session(
40+
diff_content,
41+
services,
42+
on_progress,
43+
verification_reuse_cache,
44+
)
45+
.await
3846
}
3947
}

src/review/pipeline/session/build.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ pub(super) async fn build_review_session(
1616
diff_content: &str,
1717
services: &PipelineServices,
1818
on_progress: Option<ProgressCallback>,
19+
verification_reuse_cache: crate::review::verification::VerificationReuseCache,
1920
) -> Result<ReviewSession> {
2021
let diffs = parse_review_diffs(diff_content, services)?;
2122
let source_files = load_source_files(&diffs, services);
@@ -41,6 +42,7 @@ pub(super) async fn build_review_session(
4142
enhanced_ctx,
4243
enhanced_guidance,
4344
verification_context: HashMap::new(),
45+
verification_reuse_cache,
4446
graph_query_traces: Vec::new(),
4547
})
4648
}

0 commit comments

Comments
 (0)