Skip to content

Commit dd461fa

Browse files
committed
feat: group review findings by lifecycle section
1 parent bfcab1e commit dd461fa

File tree

3 files changed

+170
-25
lines changed

3 files changed

+170
-25
lines changed

TODO.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ This roadmap is derived from deep research into Greptile's public docs, blog, MC
8888
## 6. Review UX and Workflow Integration
8989

9090
51. [x] Add visible accepted/rejected/dismissed badges to comments throughout the UI, not just icon state.
91-
52. [ ] Add comment grouping by unresolved, fixed, stale, and informational sections in `ReviewView`.
91+
52. [x] Add comment grouping by unresolved, fixed, stale, and informational sections in `ReviewView`.
9292
53. [x] Add a "show only blockers" mode for large reviews.
9393
54. [ ] Add keyboard actions for thumbs, resolve, and jump-to-next-finding workflows.
9494
55. [x] Add file-level readiness summaries in the diff sidebar.
@@ -175,4 +175,5 @@ This roadmap is derived from deep research into Greptile's public docs, blog, MC
175175
- [x] Add learned-rules, attention-gap, and rejected-pattern analytics API endpoints for automation consumers.
176176
- [x] Add a PR re-review API that reuses stored review metadata and posting policy.
177177
- [x] Add pattern repository resolution coverage for repo-local directories, Git sources, and broken-source skips.
178+
- [x] Group ReviewView list-mode findings into unresolved, fixed, stale, and informational sections.
178179
- [ ] Commit and push each validated checkpoint before moving to the next epic.

web/src/pages/ReviewView.tsx

Lines changed: 132 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,48 @@ type ReviewCommentFilters = {
2424
blockerOnly?: boolean
2525
}
2626

27+
type ReviewCommentSectionKey = 'stale' | 'unresolved' | 'informational' | 'fixed'
28+
29+
type ReviewCommentSection = {
30+
key: ReviewCommentSectionKey
31+
title: string
32+
description: string
33+
badgeClassName: string
34+
files: Array<{
35+
path: string
36+
comments: Comment[]
37+
}>
38+
}
39+
40+
const REVIEW_COMMENT_SECTION_ORDER: ReviewCommentSectionKey[] = ['stale', 'unresolved', 'informational', 'fixed']
41+
42+
const REVIEW_COMMENT_SECTION_META: Record<ReviewCommentSectionKey, Omit<ReviewCommentSection, 'files'>> = {
43+
stale: {
44+
key: 'stale',
45+
title: 'Stale',
46+
description: 'Open findings from a review that predates newer commits.',
47+
badgeClassName: 'bg-accent/10 text-accent border border-accent/20',
48+
},
49+
unresolved: {
50+
key: 'unresolved',
51+
title: 'Unresolved',
52+
description: 'Open blocking findings that still need action.',
53+
badgeClassName: 'bg-sev-warning/10 text-sev-warning border border-sev-warning/20',
54+
},
55+
informational: {
56+
key: 'informational',
57+
title: 'Informational',
58+
description: 'Open non-blocking findings worth keeping in view.',
59+
badgeClassName: 'bg-surface-2 text-text-muted border border-border',
60+
},
61+
fixed: {
62+
key: 'fixed',
63+
title: 'Fixed',
64+
description: 'Resolved and dismissed findings retained for audit history.',
65+
badgeClassName: 'bg-sev-suggestion/10 text-sev-suggestion border border-sev-suggestion/20',
66+
},
67+
}
68+
2769
function normalizeCommentFilePath(filePath: string): string {
2870
return filePath.replace(/^\.\//, '')
2971
}
@@ -32,6 +74,57 @@ function isBlockingComment(comment: Pick<Comment, 'severity' | 'status'>): boole
3274
return (comment.status ?? 'Open') === 'Open' && BLOCKING_SEVERITIES.has(comment.severity)
3375
}
3476

77+
function classifyReviewCommentSection(
78+
comment: Comment,
79+
mergeReadiness?: MergeReadiness,
80+
): ReviewCommentSectionKey {
81+
const status = comment.status ?? 'Open'
82+
if (status === 'Resolved' || status === 'Dismissed') {
83+
return 'fixed'
84+
}
85+
86+
if (mergeReadiness === 'NeedsReReview') {
87+
return 'stale'
88+
}
89+
90+
return BLOCKING_SEVERITIES.has(comment.severity) ? 'unresolved' : 'informational'
91+
}
92+
93+
function buildReviewCommentSections(
94+
comments: Comment[],
95+
mergeReadiness?: MergeReadiness,
96+
): ReviewCommentSection[] {
97+
const groupedSections = new Map<ReviewCommentSectionKey, Map<string, Comment[]>>()
98+
99+
for (const comment of comments) {
100+
const sectionKey = classifyReviewCommentSection(comment, mergeReadiness)
101+
if (!groupedSections.has(sectionKey)) {
102+
groupedSections.set(sectionKey, new Map())
103+
}
104+
105+
const sectionFiles = groupedSections.get(sectionKey)!
106+
if (!sectionFiles.has(comment.file_path)) {
107+
sectionFiles.set(comment.file_path, [])
108+
}
109+
sectionFiles.get(comment.file_path)!.push(comment)
110+
}
111+
112+
return REVIEW_COMMENT_SECTION_ORDER.flatMap((sectionKey) => {
113+
const sectionFiles = groupedSections.get(sectionKey)
114+
if (!sectionFiles || sectionFiles.size === 0) {
115+
return []
116+
}
117+
118+
return [{
119+
...REVIEW_COMMENT_SECTION_META[sectionKey],
120+
files: [...sectionFiles.entries()].map(([path, sectionComments]) => ({
121+
path,
122+
comments: sectionComments,
123+
})),
124+
}]
125+
})
126+
}
127+
35128
function filterReviewComments(
36129
comments: Comment[],
37130
{
@@ -105,6 +198,11 @@ export function ReviewView() {
105198
blockerOnly: showOnlyBlockers,
106199
}), [comments, severityFilter, activeSelectedFile, categoryFilter, lifecycleFilter, showOnlyBlockers])
107200

201+
const commentSections = useMemo(
202+
() => buildReviewCommentSections(filteredComments, review?.summary?.merge_readiness),
203+
[filteredComments, review?.summary?.merge_readiness],
204+
)
205+
108206
// All hooks MUST be above this line — no hooks after early returns
109207

110208
if (isLoading || !review) {
@@ -188,14 +286,6 @@ export function ReviewView() {
188286
lifecycle.mutate({ commentId, status })
189287
}
190288

191-
// Group comments by file for list view (no useMemo — filteredComments changes every render)
192-
const groupedByFile = new Map<string, typeof filteredComments>()
193-
for (const c of filteredComments) {
194-
const key = c.file_path
195-
if (!groupedByFile.has(key)) groupedByFile.set(key, [])
196-
groupedByFile.get(key)!.push(c)
197-
}
198-
199289
const visibleDiffFiles = activeSelectedFile
200290
? filteredDiffFiles.filter(f => f.path === activeSelectedFile)
201291
: filteredDiffFiles
@@ -491,26 +581,44 @@ export function ReviewView() {
491581
) : (
492582
/* List view / fallback when no diff content */
493583
<div className="space-y-4 max-w-3xl">
494-
{[...groupedByFile.entries()].map(([file, fileComments]) => (
495-
<div key={file}>
496-
<div className="flex items-center gap-2 mb-2">
497-
<FileCode size={13} className="text-text-muted" />
498-
<span className="font-code text-[12px] text-text-muted">{file.split('/').slice(0, -1).join('/')}/</span>
499-
<span className="font-code text-[12px] text-text-primary font-medium">{file.split('/').pop()}</span>
584+
{commentSections.map((section) => (
585+
<section key={section.key} className="border border-border rounded-lg overflow-hidden bg-surface-1/60">
586+
<div className="px-4 py-3 border-b border-border bg-surface-2/70 flex items-start justify-between gap-3">
587+
<div className="min-w-0">
588+
<div className="flex items-center gap-2">
589+
<h2 className="text-[12px] font-semibold text-text-primary">{section.title}</h2>
590+
<span className={`text-[10px] px-2 py-0.5 rounded font-code ${section.badgeClassName}`}>
591+
{section.files.reduce((total, file) => total + file.comments.length, 0)}
592+
</span>
593+
</div>
594+
<p className="mt-1 text-[11px] text-text-muted">{section.description}</p>
595+
</div>
500596
</div>
501-
<div className="space-y-2 ml-5">
502-
{fileComments.map(comment => (
503-
<div key={comment.id}>
504-
<span className="text-[10px] text-text-muted font-code">L{comment.line_number}</span>
505-
<CommentCard
506-
comment={comment}
507-
onFeedback={action => handleFeedback(comment.id, action)}
508-
onLifecycleChange={status => handleLifecycleChange(comment.id, status)}
509-
/>
597+
598+
<div className="p-4 space-y-4">
599+
{section.files.map(({ path, comments: fileComments }) => (
600+
<div key={`${section.key}-${path}`}>
601+
<div className="flex items-center gap-2 mb-2">
602+
<FileCode size={13} className="text-text-muted" />
603+
<span className="font-code text-[12px] text-text-muted">{path.split('/').slice(0, -1).join('/')}/</span>
604+
<span className="font-code text-[12px] text-text-primary font-medium">{path.split('/').pop()}</span>
605+
</div>
606+
<div className="space-y-2 ml-5">
607+
{fileComments.map(comment => (
608+
<div key={comment.id}>
609+
<span className="text-[10px] text-text-muted font-code">L{comment.line_number}</span>
610+
<CommentCard
611+
comment={comment}
612+
onFeedback={action => handleFeedback(comment.id, action)}
613+
onLifecycleChange={status => handleLifecycleChange(comment.id, status)}
614+
/>
615+
</div>
616+
))}
617+
</div>
510618
</div>
511619
))}
512620
</div>
513-
</div>
621+
</section>
514622
))}
515623

516624
{filteredComments.length === 0 && (

web/src/pages/__tests__/ReviewView.test.tsx

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,42 @@ describe('ReviewView blocker mode', () => {
176176
expect(screen.getByText('No open blockers remain in this review.')).toBeInTheDocument()
177177
})
178178

179+
it('groups list view comments into unresolved, informational, and fixed sections', async () => {
180+
const user = userEvent.setup()
181+
useReviewMock.mockReturnValue({ data: makeReview(), isLoading: false })
182+
183+
render(<ReviewView />)
184+
185+
await user.click(screen.getByRole('button', { name: 'List' }))
186+
187+
expect(screen.getByRole('heading', { name: 'Unresolved' })).toBeInTheDocument()
188+
expect(screen.getByRole('heading', { name: 'Informational' })).toBeInTheDocument()
189+
expect(screen.getByRole('heading', { name: 'Fixed' })).toBeInTheDocument()
190+
expect(screen.queryByRole('heading', { name: 'Stale' })).not.toBeInTheDocument()
191+
})
192+
193+
it('groups open comments into a stale section when the review needs re-review', async () => {
194+
const user = userEvent.setup()
195+
useReviewMock.mockReturnValue({
196+
data: makeReview({
197+
summary: makeSummary({
198+
merge_readiness: 'NeedsReReview',
199+
readiness_reasons: ['New commits landed after the latest completed review.'],
200+
}),
201+
}),
202+
isLoading: false,
203+
})
204+
205+
render(<ReviewView />)
206+
207+
await user.click(screen.getByRole('button', { name: 'List' }))
208+
209+
expect(screen.getByRole('heading', { name: 'Stale' })).toBeInTheDocument()
210+
expect(screen.queryByRole('heading', { name: 'Unresolved' })).not.toBeInTheDocument()
211+
expect(screen.queryByRole('heading', { name: 'Informational' })).not.toBeInTheDocument()
212+
expect(screen.getByRole('heading', { name: 'Fixed' })).toBeInTheDocument()
213+
})
214+
179215
it('shows a train-the-reviewer callout when thumbs coverage is low', () => {
180216
useReviewMock.mockReturnValue({ data: makeReview(), isLoading: false })
181217

0 commit comments

Comments
 (0)