Skip to content

Commit 70fdb54

Browse files
haasonsaashaasonsaas
committed
feat: add finding lifecycle and merge readiness
Give review findings explicit open, resolved, and dismissed state so teams can track what still blocks a change after the initial model pass. Surface merge readiness from unresolved blockers across the API, summaries, and review UI so reviews can converge beyond thumbs alone. Made-with: Cursor
1 parent 0729414 commit 70fdb54

File tree

32 files changed

+587
-46
lines changed

32 files changed

+587
-46
lines changed

‎TODO.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ This roadmap is derived from deep research into Greptile's public docs, blog, MC
3535

3636
## 2. Review Lifecycle and Merge Readiness
3737

38-
11. [ ] Track unresolved vs resolved findings for PR reviews as a first-class lifecycle state.
38+
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.
4040
13. [ ] 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.
@@ -155,4 +155,5 @@ This roadmap is derived from deep research into Greptile's public docs, blog, MC
155155
- [x] Rewrite this roadmap into the active backlog and keep it updated as slices ship.
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.
158+
- [x] Ship first-pass finding lifecycle state and lightweight merge readiness through the backend, API, CLI summaries, and review UI.
158159
- [ ] Commit and push each validated checkpoint before moving to the next epic.

‎migrations/002_comment_lifecycle.sql‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTER TABLE comments
2+
ADD COLUMN IF NOT EXISTS lifecycle_status TEXT NOT NULL DEFAULT 'Open';

‎src/commands/eval/pattern/matching/run.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ mod tests {
5252
tags: vec!["reliability".to_string(), "panic".to_string()],
5353
fix_effort: FixEffort::Low,
5454
feedback: None,
55+
status: crate::core::comment::CommentStatus::Open,
5556
};
5657

5758
let pattern = EvalPattern {
@@ -83,6 +84,7 @@ mod tests {
8384
tags: vec!["path-traversal".to_string()],
8485
fix_effort: FixEffort::Medium,
8586
feedback: None,
87+
status: crate::core::comment::CommentStatus::Open,
8688
};
8789

8890
let pattern = EvalPattern {
@@ -113,6 +115,7 @@ mod tests {
113115
tags: vec!["broken-access-control".to_string(), "cwe-284".to_string()],
114116
fix_effort: FixEffort::Low,
115117
feedback: None,
118+
status: crate::core::comment::CommentStatus::Open,
116119
};
117120

118121
let pattern = EvalPattern {
@@ -141,6 +144,7 @@ mod tests {
141144
tags: vec!["authorization-bypass".to_string(), "async-await".to_string()],
142145
fix_effort: FixEffort::Low,
143146
feedback: None,
147+
status: crate::core::comment::CommentStatus::Open,
144148
};
145149

146150
let pattern = EvalPattern {
@@ -173,6 +177,7 @@ mod tests {
173177
tags: vec!["silent-failure".to_string()],
174178
fix_effort: FixEffort::Low,
175179
feedback: None,
180+
status: crate::core::comment::CommentStatus::Open,
176181
};
177182

178183
let pattern = EvalPattern {
@@ -201,6 +206,7 @@ mod tests {
201206
tags: vec!["information-disclosure".to_string(), "cwe-209".to_string()],
202207
fix_effort: FixEffort::Low,
203208
feedback: None,
209+
status: crate::core::comment::CommentStatus::Open,
204210
};
205211

206212
let pattern = EvalPattern {
@@ -231,6 +237,7 @@ mod tests {
231237
tags: vec!["async".to_string(), "concurrency".to_string()],
232238
fix_effort: FixEffort::Low,
233239
feedback: None,
240+
status: crate::core::comment::CommentStatus::Open,
234241
};
235242

236243
let pattern = EvalPattern {
@@ -260,6 +267,7 @@ mod tests {
260267
tags: vec!["async-await".to_string(), "promise".to_string()],
261268
fix_effort: FixEffort::Low,
262269
feedback: None,
270+
status: crate::core::comment::CommentStatus::Open,
263271
};
264272

265273
let pattern = EvalPattern {
@@ -293,6 +301,7 @@ mod tests {
293301
tags: vec!["multi-tenancy".to_string(), "authorization".to_string()],
294302
fix_effort: FixEffort::Low,
295303
feedback: None,
304+
status: crate::core::comment::CommentStatus::Open,
296305
};
297306

298307
let pattern = EvalPattern {
@@ -325,6 +334,7 @@ mod tests {
325334
tags: vec!["supply-chain".to_string()],
326335
fix_effort: FixEffort::Medium,
327336
feedback: None,
337+
status: crate::core::comment::CommentStatus::Open,
328338
};
329339

330340
let pattern = EvalPattern {
@@ -360,6 +370,7 @@ mod tests {
360370
tags: vec!["supply-chain".to_string()],
361371
fix_effort: FixEffort::Medium,
362372
feedback: None,
373+
status: crate::core::comment::CommentStatus::Open,
363374
};
364375

365376
let pattern = EvalPattern {
@@ -396,6 +407,7 @@ mod tests {
396407
tags: vec!["supply-chain".to_string(), "code-execution".to_string()],
397408
fix_effort: FixEffort::Low,
398409
feedback: None,
410+
status: crate::core::comment::CommentStatus::Open,
399411
};
400412

401413
let pattern = EvalPattern {

‎src/commands/feedback_eval/input.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ mod tests {
3838
tags: vec![],
3939
fix_effort: FixEffort::Low,
4040
feedback: feedback.map(str::to_string),
41+
status: crate::core::comment::CommentStatus::Open,
4142
}
4243
}
4344

‎src/commands/misc/discussion/selection/rules.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ mod tests {
6464
tags: vec![],
6565
fix_effort: core::comment::FixEffort::Low,
6666
feedback: None,
67+
status: crate::core::comment::CommentStatus::Open,
6768
};
6869
let result = select_discussion_comment(std::slice::from_ref(&comment), None, None).unwrap();
6970
assert_eq!(result.id, "cmt_1");

‎src/commands/misc/feedback/apply/accept.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ mod tests {
3636
tags: vec![],
3737
fix_effort: core::comment::FixEffort::Low,
3838
feedback: None,
39+
status: crate::core::comment::CommentStatus::Open,
3940
};
4041

4142
let comments = vec![comment];

‎src/core/comment.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ use tags::extract_tags;
3232

3333
pub use identity::compute_comment_id;
3434
pub use types::{
35-
Category, CodeSuggestion, Comment, FixEffort, RawComment, ReviewSummary, Severity,
35+
Category, CodeSuggestion, Comment, CommentStatus, FixEffort, MergeReadiness, RawComment,
36+
ReviewSummary, Severity,
3637
};
3738

3839
pub struct CommentSynthesizer;
@@ -95,6 +96,7 @@ impl CommentSynthesizer {
9596
tags,
9697
fix_effort,
9798
feedback: None,
99+
status: CommentStatus::Open,
98100
})
99101
}
100102
}

‎src/core/comment/summary.rs‎

Lines changed: 119 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
use std::collections::{HashMap, HashSet};
22

3-
use super::{Category, Comment, ReviewSummary, Severity};
3+
use super::{Category, Comment, CommentStatus, MergeReadiness, ReviewSummary, Severity};
44

55
pub(super) fn generate_summary(comments: &[Comment]) -> ReviewSummary {
66
let mut by_severity = HashMap::new();
77
let mut by_category = HashMap::new();
88
let mut files = HashSet::new();
99
let mut critical_issues = 0;
10+
let mut open_comments = 0;
11+
let mut resolved_comments = 0;
12+
let mut dismissed_comments = 0;
13+
let mut open_blockers = 0;
1014

1115
for comment in comments {
1216
let severity_str = comment.severity.to_string();
@@ -20,6 +24,17 @@ pub(super) fn generate_summary(comments: &[Comment]) -> ReviewSummary {
2024
if matches!(comment.severity, Severity::Error) {
2125
critical_issues += 1;
2226
}
27+
28+
match comment.status {
29+
CommentStatus::Open => {
30+
open_comments += 1;
31+
if matches!(comment.severity, Severity::Error | Severity::Warning) {
32+
open_blockers += 1;
33+
}
34+
}
35+
CommentStatus::Resolved => resolved_comments += 1,
36+
CommentStatus::Dismissed => dismissed_comments += 1,
37+
}
2338
}
2439

2540
ReviewSummary {
@@ -30,6 +45,15 @@ pub(super) fn generate_summary(comments: &[Comment]) -> ReviewSummary {
3045
files_reviewed: files.len(),
3146
overall_score: calculate_overall_score(comments),
3247
recommendations: generate_recommendations(comments),
48+
open_comments,
49+
resolved_comments,
50+
dismissed_comments,
51+
open_blockers,
52+
merge_readiness: if open_blockers == 0 {
53+
MergeReadiness::Ready
54+
} else {
55+
MergeReadiness::NeedsAttention
56+
},
3357
}
3458
}
3559

@@ -59,6 +83,9 @@ fn generate_recommendations(comments: &[Comment]) -> Vec<String> {
5983
let mut style_count = 0;
6084

6185
for comment in comments {
86+
if comment.status != CommentStatus::Open {
87+
continue;
88+
}
6289
match comment.category {
6390
Category::Security => security_count += 1,
6491
Category::Performance => performance_count += 1,
@@ -85,3 +112,94 @@ fn generate_recommendations(comments: &[Comment]) -> Vec<String> {
85112

86113
recommendations
87114
}
115+
116+
#[cfg(test)]
117+
mod tests {
118+
use std::path::PathBuf;
119+
120+
use super::*;
121+
use crate::core::comment::{Category, FixEffort};
122+
123+
fn make_comment(
124+
id: &str,
125+
severity: Severity,
126+
category: Category,
127+
status: CommentStatus,
128+
) -> Comment {
129+
Comment {
130+
id: id.to_string(),
131+
file_path: PathBuf::from("src/lib.rs"),
132+
line_number: 10,
133+
content: "test".to_string(),
134+
rule_id: None,
135+
severity,
136+
category,
137+
suggestion: None,
138+
confidence: 0.9,
139+
code_suggestion: None,
140+
tags: Vec::new(),
141+
fix_effort: FixEffort::Low,
142+
feedback: None,
143+
status,
144+
}
145+
}
146+
147+
#[test]
148+
fn summary_tracks_lifecycle_and_merge_readiness() {
149+
let comments = vec![
150+
make_comment(
151+
"open-error",
152+
Severity::Error,
153+
Category::Security,
154+
CommentStatus::Open,
155+
),
156+
make_comment(
157+
"resolved-warning",
158+
Severity::Warning,
159+
Category::Bug,
160+
CommentStatus::Resolved,
161+
),
162+
make_comment(
163+
"dismissed-info",
164+
Severity::Info,
165+
Category::Style,
166+
CommentStatus::Dismissed,
167+
),
168+
];
169+
170+
let summary = generate_summary(&comments);
171+
assert_eq!(summary.total_comments, 3);
172+
assert_eq!(summary.open_comments, 1);
173+
assert_eq!(summary.resolved_comments, 1);
174+
assert_eq!(summary.dismissed_comments, 1);
175+
assert_eq!(summary.open_blockers, 1);
176+
assert_eq!(summary.merge_readiness, MergeReadiness::NeedsAttention);
177+
assert_eq!(
178+
summary.recommendations,
179+
vec!["Address 1 security issue(s) immediately".to_string()]
180+
);
181+
}
182+
183+
#[test]
184+
fn summary_is_ready_when_only_resolved_or_dismissed_comments_remain() {
185+
let comments = vec![
186+
make_comment(
187+
"resolved-error",
188+
Severity::Error,
189+
Category::Security,
190+
CommentStatus::Resolved,
191+
),
192+
make_comment(
193+
"dismissed-warning",
194+
Severity::Warning,
195+
Category::Bug,
196+
CommentStatus::Dismissed,
197+
),
198+
];
199+
200+
let summary = generate_summary(&comments);
201+
assert_eq!(summary.open_blockers, 0);
202+
assert_eq!(summary.merge_readiness, MergeReadiness::Ready);
203+
assert!(summary.recommendations.is_empty());
204+
}
205+
}

‎src/core/comment/types.rs‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ pub struct Comment {
2020
pub fix_effort: FixEffort,
2121
#[serde(default)]
2222
pub feedback: Option<String>,
23+
#[serde(default)]
24+
pub status: CommentStatus,
2325
}
2426

2527
#[derive(Debug, Clone, Serialize, Deserialize)]
@@ -39,6 +41,61 @@ pub struct ReviewSummary {
3941
pub files_reviewed: usize,
4042
pub overall_score: f32,
4143
pub recommendations: Vec<String>,
44+
#[serde(default)]
45+
pub open_comments: usize,
46+
#[serde(default)]
47+
pub resolved_comments: usize,
48+
#[serde(default)]
49+
pub dismissed_comments: usize,
50+
#[serde(default)]
51+
pub open_blockers: usize,
52+
#[serde(default)]
53+
pub merge_readiness: MergeReadiness,
54+
}
55+
56+
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Default)]
57+
pub enum CommentStatus {
58+
#[default]
59+
Open,
60+
Resolved,
61+
Dismissed,
62+
}
63+
64+
impl std::fmt::Display for CommentStatus {
65+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
66+
match self {
67+
CommentStatus::Open => write!(f, "Open"),
68+
CommentStatus::Resolved => write!(f, "Resolved"),
69+
CommentStatus::Dismissed => write!(f, "Dismissed"),
70+
}
71+
}
72+
}
73+
74+
impl CommentStatus {
75+
pub fn from_api_str(value: &str) -> Option<Self> {
76+
match value.trim().to_ascii_lowercase().as_str() {
77+
"open" => Some(Self::Open),
78+
"resolved" => Some(Self::Resolved),
79+
"dismissed" => Some(Self::Dismissed),
80+
_ => None,
81+
}
82+
}
83+
}
84+
85+
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Default)]
86+
pub enum MergeReadiness {
87+
Ready,
88+
#[default]
89+
NeedsAttention,
90+
}
91+
92+
impl std::fmt::Display for MergeReadiness {
93+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
94+
match self {
95+
MergeReadiness::Ready => write!(f, "Ready"),
96+
MergeReadiness::NeedsAttention => write!(f, "Needs attention"),
97+
}
98+
}
4299
}
43100

44101
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]

‎src/core/composable_pipeline.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,7 @@ mod tests {
373373
tags: vec![],
374374
fix_effort: FixEffort::Medium,
375375
feedback: None,
376+
status: crate::core::comment::CommentStatus::Open,
376377
}
377378
}
378379

0 commit comments

Comments
 (0)