Skip to content

Commit d5a095e

Browse files
committed
feat: separate blocking and informational findings
Split open review findings into blocking and informational buckets so merge readiness surfaces can highlight true blockers and the review detail page can call out unresolved error and warning findings explicitly. Made-with: Cursor
1 parent 60dcf63 commit d5a095e

File tree

8 files changed

+162
-6
lines changed

8 files changed

+162
-6
lines changed

TODO.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ This roadmap is derived from deep research into Greptile's public docs, blog, MC
4040
13. [x] Compute merge-readiness summaries for GitHub PR reviews using severity, unresolved count, and verification state.
4141
14. [x] Add stale-review detection when new commits land after the latest completed review.
4242
15. [x] Show "needs re-review" state in review detail and history pages for incremental PR workflows.
43-
16. [ ] Distinguish informational findings from blocking findings in lifecycle and readiness calculations.
44-
17. [ ] Add "critical blockers" summary cards for unresolved `Error` and `Warning` comments.
43+
16. [x] Distinguish informational findings from blocking findings in lifecycle and readiness calculations.
44+
17. [x] Add "critical blockers" summary cards for unresolved `Error` and `Warning` comments.
4545
18. [ ] Add per-PR readiness timelines showing when a review became mergeable.
4646
19. [ ] Store resolution timestamps for findings so mean-time-to-fix can be measured.
4747
20. [ ] Add CLI and API surfaces to query PR readiness without opening the web UI.
@@ -158,4 +158,5 @@ This roadmap is derived from deep research into Greptile's public docs, blog, MC
158158
- [x] Ship first-pass finding lifecycle state and lightweight merge readiness through the backend, API, CLI summaries, and review UI.
159159
- [x] Make merge readiness verification-aware and surface stale PR reviews as needs re-review in history/detail views.
160160
- [x] Make stale-review detection compare PR head SHAs so same-head reruns do not look stale.
161+
- [x] Split open findings into blocking vs informational buckets and surface critical blocker cards in review detail.
161162
- [ ] Commit and push each validated checkpoint before moving to the next epic.

src/core/comment/summary.rs

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,12 @@ use super::{
88
pub(super) fn generate_summary(comments: &[Comment]) -> ReviewSummary {
99
let mut by_severity = HashMap::new();
1010
let mut by_category = HashMap::new();
11+
let mut open_by_severity = HashMap::new();
1112
let mut files = HashSet::new();
1213
let mut critical_issues = 0;
1314
let mut open_comments = 0;
15+
let mut open_blocking_comments = 0;
16+
let mut open_informational_comments = 0;
1417
let mut resolved_comments = 0;
1518
let mut dismissed_comments = 0;
1619
let mut open_blockers = 0;
@@ -31,9 +34,16 @@ pub(super) fn generate_summary(comments: &[Comment]) -> ReviewSummary {
3134
match comment.status {
3235
CommentStatus::Open => {
3336
open_comments += 1;
34-
if matches!(comment.severity, Severity::Error | Severity::Warning) {
37+
*open_by_severity
38+
.entry(comment.severity.to_string())
39+
.or_insert(0) += 1;
40+
if comment.severity.is_blocking() {
41+
open_blocking_comments += 1;
3542
open_blockers += 1;
3643
}
44+
if comment.severity.is_informational() {
45+
open_informational_comments += 1;
46+
}
3747
}
3848
CommentStatus::Resolved => resolved_comments += 1,
3949
CommentStatus::Dismissed => dismissed_comments += 1,
@@ -49,6 +59,9 @@ pub(super) fn generate_summary(comments: &[Comment]) -> ReviewSummary {
4959
overall_score: calculate_overall_score(comments),
5060
recommendations: generate_recommendations(comments),
5161
open_comments,
62+
open_by_severity,
63+
open_blocking_comments,
64+
open_informational_comments,
5265
resolved_comments,
5366
dismissed_comments,
5467
open_blockers,
@@ -220,9 +233,12 @@ mod tests {
220233
let summary = generate_summary(&comments);
221234
assert_eq!(summary.total_comments, 3);
222235
assert_eq!(summary.open_comments, 1);
236+
assert_eq!(summary.open_blocking_comments, 1);
237+
assert_eq!(summary.open_informational_comments, 0);
223238
assert_eq!(summary.resolved_comments, 1);
224239
assert_eq!(summary.dismissed_comments, 1);
225240
assert_eq!(summary.open_blockers, 1);
241+
assert_eq!(summary.open_by_severity.get("Error"), Some(&1));
226242
assert_eq!(summary.merge_readiness, MergeReadiness::NeedsAttention);
227243
assert_eq!(
228244
summary.recommendations,
@@ -249,10 +265,44 @@ mod tests {
249265

250266
let summary = generate_summary(&comments);
251267
assert_eq!(summary.open_blockers, 0);
268+
assert_eq!(summary.open_blocking_comments, 0);
269+
assert_eq!(summary.open_informational_comments, 0);
252270
assert_eq!(summary.merge_readiness, MergeReadiness::Ready);
253271
assert!(summary.recommendations.is_empty());
254272
}
255273

274+
#[test]
275+
fn summary_distinguishes_blocking_and_informational_open_findings() {
276+
let comments = vec![
277+
make_comment(
278+
"open-warning",
279+
Severity::Warning,
280+
Category::Bug,
281+
CommentStatus::Open,
282+
),
283+
make_comment(
284+
"open-info",
285+
Severity::Info,
286+
Category::Documentation,
287+
CommentStatus::Open,
288+
),
289+
make_comment(
290+
"open-suggestion",
291+
Severity::Suggestion,
292+
Category::Style,
293+
CommentStatus::Open,
294+
),
295+
];
296+
297+
let summary = generate_summary(&comments);
298+
assert_eq!(summary.open_comments, 3);
299+
assert_eq!(summary.open_blocking_comments, 1);
300+
assert_eq!(summary.open_informational_comments, 2);
301+
assert_eq!(summary.open_by_severity.get("Warning"), Some(&1));
302+
assert_eq!(summary.open_by_severity.get("Info"), Some(&1));
303+
assert_eq!(summary.open_by_severity.get("Suggestion"), Some(&1));
304+
}
305+
256306
#[test]
257307
fn summary_needs_rereview_when_verification_is_inconclusive() {
258308
let comments = vec![make_comment(

src/core/comment/types.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ pub struct ReviewSummary {
4444
#[serde(default)]
4545
pub open_comments: usize,
4646
#[serde(default)]
47+
pub open_by_severity: HashMap<String, usize>,
48+
#[serde(default)]
49+
pub open_blocking_comments: usize,
50+
#[serde(default)]
51+
pub open_informational_comments: usize,
52+
#[serde(default)]
4753
pub resolved_comments: usize,
4854
#[serde(default)]
4955
pub dismissed_comments: usize,
@@ -167,6 +173,14 @@ impl Severity {
167173
Severity::Suggestion => "suggestion",
168174
}
169175
}
176+
177+
pub fn is_blocking(&self) -> bool {
178+
matches!(self, Severity::Error | Severity::Warning)
179+
}
180+
181+
pub fn is_informational(&self) -> bool {
182+
matches!(self, Severity::Info | Severity::Suggestion)
183+
}
170184
}
171185

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

src/output/format.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,10 @@ 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+
"🚧 **Blocking Open:** {} | 💡 **Informational Open:** {}\n\n",
96+
summary.open_blocking_comments, summary.open_informational_comments
97+
));
9498
output.push_str(&format!(
9599
"🧪 **Verification:** {}",
96100
summary.verification.state
@@ -307,6 +311,10 @@ pub fn format_smart_review_output(
307311
"⛔ **Open Blockers:** {}\n\n",
308312
summary.open_blockers
309313
));
314+
output.push_str(&format!(
315+
"🚧 **Blocking Open:** {} | 💡 **Informational Open:** {}\n\n",
316+
summary.open_blocking_comments, summary.open_informational_comments
317+
));
310318
output.push_str(&format!(
311319
"🧪 **Verification:** {}",
312320
summary.verification.state
@@ -899,6 +907,12 @@ mod tests {
899907
by_category: std::collections::HashMap::from([("Bug".to_string(), 2)]),
900908
recommendations: vec!["Fix bugs".to_string()],
901909
open_comments: 2,
910+
open_by_severity: std::collections::HashMap::from([
911+
("Error".to_string(), 1),
912+
("Warning".to_string(), 1),
913+
]),
914+
open_blocking_comments: 2,
915+
open_informational_comments: 0,
902916
resolved_comments: 0,
903917
dismissed_comments: 0,
904918
open_blockers: 2,

src/review/rule_helpers/reporting/summary.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ 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!(
27+
"- Blocking open: {} | Informational open: {}\n",
28+
summary.open_blocking_comments, summary.open_informational_comments
29+
));
2630
body.push_str(&format!("- Verification: {}", summary.verification.state));
2731
if summary.verification.judge_count > 0 {
2832
body.push_str(&format!(

src/server/storage_json.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,9 @@ mod tests {
533533
overall_score: 8.0,
534534
recommendations: vec!["Fix bugs".to_string()],
535535
open_comments: 2,
536+
open_by_severity: HashMap::from([("Warning".to_string(), 2)]),
537+
open_blocking_comments: 2,
538+
open_informational_comments: 0,
536539
resolved_comments: 0,
537540
dismissed_comments: 0,
538541
open_blockers: 2,

web/src/api/types.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ export interface ReviewSummary {
4141
overall_score: number
4242
recommendations: string[]
4343
open_comments: number
44+
open_by_severity: Record<string, number>
45+
open_blocking_comments: number
46+
open_informational_comments: number
4447
resolved_comments: number
4548
dismissed_comments: number
4649
open_blockers: number

web/src/pages/ReviewView.tsx

Lines changed: 70 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,9 @@ export function ReviewView() {
145145
Inconclusive: 'text-accent',
146146
}
147147

148+
const openErrorCount = review.summary?.open_by_severity.Error ?? 0
149+
const openWarningCount = review.summary?.open_by_severity.Warning ?? 0
150+
148151
return (
149152
<div className="flex h-full">
150153
{/* File sidebar */}
@@ -184,10 +187,16 @@ export function ReviewView() {
184187
open
185188
</span>
186189
<span className="flex items-center gap-1">
187-
<span className={review.summary.open_blockers > 0 ? 'text-sev-warning' : 'text-sev-suggestion'}>
188-
{review.summary.open_blockers}
190+
<span className={review.summary.open_blocking_comments > 0 ? 'text-sev-warning' : 'text-sev-suggestion'}>
191+
{review.summary.open_blocking_comments}
189192
</span>
190-
blocker{review.summary.open_blockers === 1 ? '' : 's'}
193+
blocking
194+
</span>
195+
<span className="flex items-center gap-1">
196+
<span className={review.summary.open_informational_comments > 0 ? 'text-accent' : 'text-text-muted'}>
197+
{review.summary.open_informational_comments}
198+
</span>
199+
informational
191200
</span>
192201
<span className="flex items-center gap-1">
193202
<FileCode size={11} />
@@ -249,6 +258,35 @@ export function ReviewView() {
249258
</div>
250259
)}
251260

261+
{review.summary && (
262+
<div className="px-3 py-3 border-b border-border bg-surface grid grid-cols-2 lg:grid-cols-4 gap-2">
263+
<SummaryCard
264+
label="Error Blockers"
265+
value={openErrorCount}
266+
hint="Open Error findings"
267+
tone={openErrorCount > 0 ? 'error' : 'muted'}
268+
/>
269+
<SummaryCard
270+
label="Warning Blockers"
271+
value={openWarningCount}
272+
hint="Open Warning findings"
273+
tone={openWarningCount > 0 ? 'warning' : 'muted'}
274+
/>
275+
<SummaryCard
276+
label="Blocking Open"
277+
value={review.summary.open_blocking_comments}
278+
hint="Error + Warning"
279+
tone={review.summary.open_blocking_comments > 0 ? 'warning' : 'muted'}
280+
/>
281+
<SummaryCard
282+
label="Informational Open"
283+
value={review.summary.open_informational_comments}
284+
hint="Info + Suggestion"
285+
tone={review.summary.open_informational_comments > 0 ? 'accent' : 'muted'}
286+
/>
287+
</div>
288+
)}
289+
252290
{/* Toolbar */}
253291
<div className="px-3 py-2 border-b border-border bg-surface flex items-center gap-2">
254292
{/* View mode toggle */}
@@ -373,6 +411,35 @@ export function ReviewView() {
373411
)
374412
}
375413

414+
function SummaryCard(
415+
{
416+
label,
417+
value,
418+
hint,
419+
tone,
420+
}: {
421+
label: string
422+
value: number
423+
hint: string
424+
tone: 'error' | 'warning' | 'accent' | 'muted'
425+
},
426+
) {
427+
const toneStyles = {
428+
error: 'border-sev-error/20 bg-sev-error/5 text-sev-error',
429+
warning: 'border-sev-warning/20 bg-sev-warning/5 text-sev-warning',
430+
accent: 'border-accent/20 bg-accent/5 text-accent',
431+
muted: 'border-border bg-surface-1 text-text-muted',
432+
}
433+
434+
return (
435+
<div className={`rounded-lg border px-3 py-2 ${toneStyles[tone]}`}>
436+
<div className="text-[10px] font-code uppercase tracking-[0.08em]">{label}</div>
437+
<div className="mt-1 text-lg font-semibold text-text-primary">{value}</div>
438+
<div className="text-[11px]">{hint}</div>
439+
</div>
440+
)
441+
}
442+
376443
function ProgressBanner({ progress, comments }: { progress?: import('../api/types').ReviewProgress, comments: number }) {
377444
if (!progress) {
378445
return (

0 commit comments

Comments
 (0)