Skip to content

Commit 85f3af8

Browse files
committed
feat: make review readiness verification-aware
Track verification health in persisted review summaries and surface stale PR reviews as needs re-review so merge readiness stays accurate across lifecycle updates and incremental PR workflows. Made-with: Cursor
1 parent 70fdb54 commit 85f3af8

File tree

17 files changed

+504
-65
lines changed

17 files changed

+504
-65
lines changed

TODO.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,9 @@ This roadmap is derived from deep research into Greptile's public docs, blog, MC
3737

3838
11. [x] Track unresolved vs resolved findings for PR reviews as a first-class lifecycle state.
3939
12. [ ] Add review completeness metrics: total findings, acknowledged findings, fixed findings, stale findings.
40-
13. [ ] Compute merge-readiness summaries for GitHub PR reviews using severity, unresolved count, and verification state.
40+
13. [x] Compute merge-readiness summaries for GitHub PR reviews using severity, unresolved count, and verification state.
4141
14. [ ] Add stale-review detection when new commits land after the latest completed review.
42-
15. [ ] Show "needs re-review" state in review detail and history pages for incremental PR workflows.
42+
15. [x] Show "needs re-review" state in review detail and history pages for incremental PR workflows.
4343
16. [ ] Distinguish informational findings from blocking findings in lifecycle and readiness calculations.
4444
17. [ ] Add "critical blockers" summary cards for unresolved `Error` and `Warning` comments.
4545
18. [ ] Add per-PR readiness timelines showing when a review became mergeable.
@@ -156,4 +156,5 @@ This roadmap is derived from deep research into Greptile's public docs, blog, MC
156156
- [x] Productize the learning loop in Analytics with reaction coverage and acceptance trends.
157157
- [x] Surface repository rule sources and pattern repository sources in Settings.
158158
- [x] Ship first-pass finding lifecycle state and lightweight merge readiness through the backend, API, CLI summaries, and review UI.
159+
- [x] Make merge readiness verification-aware and surface stale PR reviews as needs re-review in history/detail views.
159160
- [ ] Commit and push each validated checkpoint before moving to the next epic.

src/core/comment.rs

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,17 @@ use ordering::{
2727
#[cfg(test)]
2828
use std::path::PathBuf;
2929
use suggestions::generate_code_suggestion;
30-
use summary::generate_summary as build_review_summary;
30+
use summary::{
31+
apply_review_runtime_state as apply_review_runtime_summary_state,
32+
apply_verification as apply_verification_summary, generate_summary as build_review_summary,
33+
inherit_review_state as inherit_review_summary_state,
34+
};
3135
use tags::extract_tags;
3236

3337
pub use identity::compute_comment_id;
3438
pub use types::{
3539
Category, CodeSuggestion, Comment, CommentStatus, FixEffort, MergeReadiness, RawComment,
36-
ReviewSummary, Severity,
40+
ReviewSummary, ReviewVerificationState, ReviewVerificationSummary, Severity,
3741
};
3842

3943
pub struct CommentSynthesizer;
@@ -56,6 +60,24 @@ impl CommentSynthesizer {
5660
build_review_summary(comments)
5761
}
5862

63+
pub fn inherit_review_state(
64+
summary: ReviewSummary,
65+
previous: Option<&ReviewSummary>,
66+
) -> ReviewSummary {
67+
inherit_review_summary_state(summary, previous)
68+
}
69+
70+
pub fn apply_verification(
71+
summary: ReviewSummary,
72+
verification: ReviewVerificationSummary,
73+
) -> ReviewSummary {
74+
apply_verification_summary(summary, verification)
75+
}
76+
77+
pub fn apply_runtime_review_state(summary: ReviewSummary, stale_review: bool) -> ReviewSummary {
78+
apply_review_runtime_summary_state(summary, stale_review)
79+
}
80+
5981
fn process_raw_comment(raw: RawComment) -> Result<Comment> {
6082
let lower = raw.content.to_lowercase();
6183
let severity = raw

src/core/comment/summary.rs

Lines changed: 103 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
use std::collections::{HashMap, HashSet};
22

3-
use super::{Category, Comment, CommentStatus, MergeReadiness, ReviewSummary, Severity};
3+
use super::{
4+
Category, Comment, CommentStatus, MergeReadiness, ReviewSummary, ReviewVerificationState,
5+
ReviewVerificationSummary, Severity,
6+
};
47

58
pub(super) fn generate_summary(comments: &[Comment]) -> ReviewSummary {
69
let mut by_severity = HashMap::new();
@@ -49,11 +52,58 @@ pub(super) fn generate_summary(comments: &[Comment]) -> ReviewSummary {
4952
resolved_comments,
5053
dismissed_comments,
5154
open_blockers,
52-
merge_readiness: if open_blockers == 0 {
53-
MergeReadiness::Ready
54-
} else {
55-
MergeReadiness::NeedsAttention
56-
},
55+
merge_readiness: default_merge_readiness(open_blockers),
56+
verification: ReviewVerificationSummary::default(),
57+
readiness_reasons: Vec::new(),
58+
}
59+
}
60+
61+
pub(super) fn inherit_review_state(
62+
mut summary: ReviewSummary,
63+
previous: Option<&ReviewSummary>,
64+
) -> ReviewSummary {
65+
if let Some(previous) = previous {
66+
summary.verification = previous.verification.clone();
67+
}
68+
apply_review_runtime_state(summary, false)
69+
}
70+
71+
pub(super) fn apply_verification(
72+
mut summary: ReviewSummary,
73+
verification: ReviewVerificationSummary,
74+
) -> ReviewSummary {
75+
summary.verification = verification;
76+
apply_review_runtime_state(summary, false)
77+
}
78+
79+
pub(super) fn apply_review_runtime_state(
80+
mut summary: ReviewSummary,
81+
stale_review: bool,
82+
) -> ReviewSummary {
83+
let mut reasons = Vec::new();
84+
if matches!(
85+
summary.verification.state,
86+
ReviewVerificationState::Inconclusive
87+
) {
88+
reasons.push("verification was inconclusive or fail-open; rerun this review".to_string());
89+
}
90+
if stale_review {
91+
reasons.push("a newer review exists for this pull request".to_string());
92+
}
93+
summary.readiness_reasons = reasons;
94+
summary.merge_readiness = if !summary.readiness_reasons.is_empty() {
95+
MergeReadiness::NeedsReReview
96+
} else {
97+
default_merge_readiness(summary.open_blockers)
98+
};
99+
summary
100+
}
101+
102+
fn default_merge_readiness(open_blockers: usize) -> MergeReadiness {
103+
if open_blockers == 0 {
104+
MergeReadiness::Ready
105+
} else {
106+
MergeReadiness::NeedsAttention
57107
}
58108
}
59109

@@ -202,4 +252,51 @@ mod tests {
202252
assert_eq!(summary.merge_readiness, MergeReadiness::Ready);
203253
assert!(summary.recommendations.is_empty());
204254
}
255+
256+
#[test]
257+
fn summary_needs_rereview_when_verification_is_inconclusive() {
258+
let comments = vec![make_comment(
259+
"open-warning",
260+
Severity::Warning,
261+
Category::Bug,
262+
CommentStatus::Open,
263+
)];
264+
265+
let summary = apply_verification(
266+
generate_summary(&comments),
267+
ReviewVerificationSummary {
268+
state: ReviewVerificationState::Inconclusive,
269+
judge_count: 1,
270+
required_votes: 1,
271+
warning_count: 1,
272+
filtered_comments: 0,
273+
abstained_comments: 1,
274+
},
275+
);
276+
277+
assert_eq!(summary.merge_readiness, MergeReadiness::NeedsReReview);
278+
assert_eq!(
279+
summary.verification.state,
280+
ReviewVerificationState::Inconclusive
281+
);
282+
assert_eq!(summary.readiness_reasons.len(), 1);
283+
}
284+
285+
#[test]
286+
fn stale_review_forces_needs_rereview_even_without_blockers() {
287+
let comments = vec![make_comment(
288+
"resolved-warning",
289+
Severity::Warning,
290+
Category::Bug,
291+
CommentStatus::Resolved,
292+
)];
293+
294+
let summary = apply_review_runtime_state(generate_summary(&comments), true);
295+
assert_eq!(summary.open_blockers, 0);
296+
assert_eq!(summary.merge_readiness, MergeReadiness::NeedsReReview);
297+
assert_eq!(
298+
summary.readiness_reasons,
299+
vec!["a newer review exists for this pull request".to_string()]
300+
);
301+
}
205302
}

src/core/comment/types.rs

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ pub struct ReviewSummary {
5151
pub open_blockers: usize,
5252
#[serde(default)]
5353
pub merge_readiness: MergeReadiness,
54+
#[serde(default)]
55+
pub verification: ReviewVerificationSummary,
56+
#[serde(default, skip_serializing_if = "Vec::is_empty")]
57+
pub readiness_reasons: Vec<String>,
5458
}
5559

5660
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Default)]
@@ -85,15 +89,51 @@ impl CommentStatus {
8589
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Default)]
8690
pub enum MergeReadiness {
8791
Ready,
88-
#[default]
8992
NeedsAttention,
93+
#[default]
94+
NeedsReReview,
9095
}
9196

9297
impl std::fmt::Display for MergeReadiness {
9398
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
9499
match self {
95100
MergeReadiness::Ready => write!(f, "Ready"),
96101
MergeReadiness::NeedsAttention => write!(f, "Needs attention"),
102+
MergeReadiness::NeedsReReview => write!(f, "Needs re-review"),
103+
}
104+
}
105+
}
106+
107+
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)]
108+
pub struct ReviewVerificationSummary {
109+
#[serde(default)]
110+
pub state: ReviewVerificationState,
111+
#[serde(default)]
112+
pub judge_count: usize,
113+
#[serde(default)]
114+
pub required_votes: usize,
115+
#[serde(default)]
116+
pub warning_count: usize,
117+
#[serde(default)]
118+
pub filtered_comments: usize,
119+
#[serde(default)]
120+
pub abstained_comments: usize,
121+
}
122+
123+
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Default)]
124+
pub enum ReviewVerificationState {
125+
#[default]
126+
NotApplicable,
127+
Verified,
128+
Inconclusive,
129+
}
130+
131+
impl std::fmt::Display for ReviewVerificationState {
132+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
133+
match self {
134+
ReviewVerificationState::NotApplicable => write!(f, "Not applicable"),
135+
ReviewVerificationState::Verified => write!(f, "Verified"),
136+
ReviewVerificationState::Inconclusive => write!(f, "Inconclusive"),
97137
}
98138
}
99139
}

src/output/format.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,26 @@ pub fn format_as_markdown(comments: &[core::Comment], rule_priority: &[String])
9191
"⛔ **Open Blockers:** {}\n\n",
9292
summary.open_blockers
9393
));
94+
output.push_str(&format!(
95+
"🧪 **Verification:** {}",
96+
summary.verification.state
97+
));
98+
if summary.verification.judge_count > 0 {
99+
output.push_str(&format!(
100+
" (votes {}/{}, warnings {})",
101+
summary.verification.required_votes,
102+
summary.verification.judge_count,
103+
summary.verification.warning_count
104+
));
105+
}
106+
output.push_str("\n\n");
107+
if !summary.readiness_reasons.is_empty() {
108+
output.push_str("### Review State\n\n");
109+
for reason in &summary.readiness_reasons {
110+
output.push_str(&format!("- {}\n", reason));
111+
}
112+
output.push('\n');
113+
}
94114

95115
// Severity breakdown
96116
output.push_str("### Issues by Severity\n\n");
@@ -287,6 +307,26 @@ pub fn format_smart_review_output(
287307
"⛔ **Open Blockers:** {}\n\n",
288308
summary.open_blockers
289309
));
310+
output.push_str(&format!(
311+
"🧪 **Verification:** {}",
312+
summary.verification.state
313+
));
314+
if summary.verification.judge_count > 0 {
315+
output.push_str(&format!(
316+
" (votes {}/{}, warnings {})",
317+
summary.verification.required_votes,
318+
summary.verification.judge_count,
319+
summary.verification.warning_count
320+
));
321+
}
322+
output.push_str("\n\n");
323+
if !summary.readiness_reasons.is_empty() {
324+
output.push_str("### 🔁 Review State\n\n");
325+
for reason in &summary.readiness_reasons {
326+
output.push_str(&format!("- {}\n", reason));
327+
}
328+
output.push('\n');
329+
}
290330

291331
if let Some(pr_summary) = pr_summary {
292332
output.push_str(&format_pr_summary_section(pr_summary));
@@ -863,6 +903,8 @@ mod tests {
863903
dismissed_comments: 0,
864904
open_blockers: 2,
865905
merge_readiness: crate::core::comment::MergeReadiness::NeedsAttention,
906+
verification: crate::core::comment::ReviewVerificationSummary::default(),
907+
readiness_reasons: Vec::new(),
866908
};
867909
let comments = vec![
868910
build_test_comment(

src/review/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ pub use rule_helpers::{
3333
apply_rule_overrides, build_pr_summary_comment_body, inject_rule_context, load_review_rules,
3434
normalize_rule_id, summarize_rule_hits,
3535
};
36+
#[allow(unused_imports)]
37+
pub(crate) use verification::summarize_review_verification;
3638

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

src/review/rule_helpers/reporting/summary.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,22 @@ pub fn build_pr_summary_comment_body(
2323
summary.open_comments, summary.resolved_comments, summary.dismissed_comments
2424
));
2525
body.push_str(&format!("- Open blockers: {}\n", summary.open_blockers));
26+
body.push_str(&format!("- Verification: {}", summary.verification.state));
27+
if summary.verification.judge_count > 0 {
28+
body.push_str(&format!(
29+
" (votes {}/{}, warnings {})",
30+
summary.verification.required_votes,
31+
summary.verification.judge_count,
32+
summary.verification.warning_count
33+
));
34+
}
35+
body.push('\n');
36+
if !summary.readiness_reasons.is_empty() {
37+
body.push_str("- Review state:\n");
38+
for reason in &summary.readiness_reasons {
39+
body.push_str(&format!(" - {}\n", reason));
40+
}
41+
}
2642

2743
if summary.total_comments == 0 {
2844
body.push_str("\nNo issues detected in this PR by DiffScope.\n");

src/review/verification.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,41 @@ pub struct VerificationReport {
5656
pub judges: Vec<VerificationJudgeRun>,
5757
}
5858

59+
pub(crate) fn summarize_review_verification(
60+
report: Option<&VerificationReport>,
61+
warnings: &[String],
62+
) -> crate::core::comment::ReviewVerificationSummary {
63+
let warning_count = warnings.len();
64+
match report {
65+
Some(report) => crate::core::comment::ReviewVerificationSummary {
66+
state: if warning_count > 0 {
67+
crate::core::comment::ReviewVerificationState::Inconclusive
68+
} else {
69+
crate::core::comment::ReviewVerificationState::Verified
70+
},
71+
judge_count: report.judge_count,
72+
required_votes: report.required_votes,
73+
warning_count,
74+
filtered_comments: report
75+
.judges
76+
.iter()
77+
.map(|judge| judge.filtered_comments)
78+
.sum(),
79+
abstained_comments: report
80+
.judges
81+
.iter()
82+
.map(|judge| judge.abstained_comments)
83+
.sum(),
84+
},
85+
None if warning_count > 0 => crate::core::comment::ReviewVerificationSummary {
86+
state: crate::core::comment::ReviewVerificationState::Inconclusive,
87+
warning_count,
88+
..Default::default()
89+
},
90+
None => crate::core::comment::ReviewVerificationSummary::default(),
91+
}
92+
}
93+
5994
const VERIFICATION_BATCH_SIZE: usize = 6;
6095

6196
#[derive(Debug, Clone)]

0 commit comments

Comments
 (0)