Skip to content

Commit 34cb9f6

Browse files
authored
🤖 Review panel read state tracking and visual improvements (#323)
## Overview Implements comprehensive read state tracking for code review hunks with visual improvements and workflow optimizations. ## Read State Tracking - Content-based hunk IDs (hash of file path + line range + diff content) ensure stable tracking across refreshes - LRU eviction with 1024-entry limit prevents unbounded localStorage growth - Persisted per-workspace with localStorage - Keyboard shortcut (m) to toggle read state - Auto-collapse read hunks, auto-expand unread ## Visual Improvements **Color System** - Centralized review accent color (muted yellow/orange at 75% opacity) - Blue color for read state indicators (strikethrough, borders) - Removed hover border clash on active hunks - Added anti-pattern comment for inline alpha usage **FileTree** - Strikethrough with blue color for fully-read files - Dimmed text for unknown state (when file not loaded) - Distinguished three states: unread (normal), fully read (dimmed + strikethrough), unknown (dimmed only) **Hunk Display** - Active hunk gets yellow border for selection feedback - Clicking line to comment activates parent hunk - Read hunks show checkmark and blue border - Collapsed state shows line count and read status ## Workflow Improvements **Auto-navigation** - Marking hunk as read (m) auto-advances to next unread hunk when "Show read" is off - Smart selection: next → previous → none - Scroll selected hunk into view on j/k navigation - Manual expand/collapse state respected independent of read state **Review Notes** - Keybind hints in placeholder: "j, k to iterate through hunks, m to toggle as read" - Shift-click to select line ranges - Cmd+Enter to submit, Esc to cancel ## Testing - Unit tests for LRU eviction logic - Refactored to export `evictOldestReviews` from source (no duplication in tests) _Generated with `cmux`_
1 parent 99e1f10 commit 34cb9f6

File tree

11 files changed

+542
-182
lines changed

11 files changed

+542
-182
lines changed

src/components/RightSidebar/CodeReview/FileTree.tsx

Lines changed: 98 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,42 @@ const TreeNode = styled.div<{ depth: number; isSelected: boolean }>`
3333
}
3434
`;
3535

36-
const FileName = styled.span`
36+
const FileName = styled.span<{ isFullyRead?: boolean; isUnknownState?: boolean }>`
3737
color: #ccc;
3838
flex: 1;
39+
${(props) =>
40+
props.isFullyRead &&
41+
`
42+
color: #666;
43+
text-decoration: line-through;
44+
text-decoration-color: var(--color-read);
45+
text-decoration-thickness: 2px;
46+
`}
47+
${(props) =>
48+
props.isUnknownState &&
49+
!props.isFullyRead &&
50+
`
51+
color: #666;
52+
`}
3953
`;
4054

41-
const DirectoryName = styled.span`
55+
const DirectoryName = styled.span<{ isFullyRead?: boolean; isUnknownState?: boolean }>`
4256
color: #888;
4357
flex: 1;
58+
${(props) =>
59+
props.isFullyRead &&
60+
`
61+
color: #666;
62+
text-decoration: line-through;
63+
text-decoration-color: var(--color-read);
64+
text-decoration-thickness: 2px;
65+
`}
66+
${(props) =>
67+
props.isUnknownState &&
68+
!props.isFullyRead &&
69+
`
70+
color: #666;
71+
`}
4472
`;
4573

4674
const DirectoryStats = styled.span<{ isOpen: boolean }>`
@@ -117,13 +145,56 @@ const EmptyState = styled.div`
117145
text-align: center;
118146
`;
119147

148+
/**
149+
* Compute read status for a directory by recursively checking all descendant files
150+
* Returns "fully-read" if all files are fully read, "unknown" if any file has unknown status, null otherwise
151+
*/
152+
function computeDirectoryReadStatus(
153+
node: FileTreeNode,
154+
getFileReadStatus?: (filePath: string) => { total: number; read: number } | null
155+
): "fully-read" | "unknown" | null {
156+
if (!node.isDirectory || !getFileReadStatus) return null;
157+
158+
let hasUnknown = false;
159+
let fileCount = 0;
160+
let fullyReadCount = 0;
161+
162+
const checkNode = (n: FileTreeNode) => {
163+
if (n.isDirectory) {
164+
// Recurse into children
165+
n.children.forEach(checkNode);
166+
} else {
167+
// Check file status
168+
fileCount++;
169+
const status = getFileReadStatus(n.path);
170+
if (status === null) {
171+
hasUnknown = true;
172+
} else if (status.read === status.total && status.total > 0) {
173+
fullyReadCount++;
174+
}
175+
}
176+
};
177+
178+
checkNode(node);
179+
180+
// If any file has unknown state, directory is unknown
181+
if (hasUnknown) return "unknown";
182+
183+
// If all files are fully read, directory is fully read
184+
if (fileCount > 0 && fullyReadCount === fileCount) return "fully-read";
185+
186+
// Otherwise, directory has partial/no read state
187+
return null;
188+
}
189+
120190
const TreeNodeContent: React.FC<{
121191
node: FileTreeNode;
122192
depth: number;
123193
selectedPath: string | null;
124194
onSelectFile: (path: string | null) => void;
125195
commonPrefix: string | null;
126-
}> = ({ node, depth, selectedPath, onSelectFile, commonPrefix }) => {
196+
getFileReadStatus?: (filePath: string) => { total: number; read: number } | null;
197+
}> = ({ node, depth, selectedPath, onSelectFile, commonPrefix, getFileReadStatus }) => {
127198
const [isOpen, setIsOpen] = useState(depth < 2); // Auto-expand first 2 levels
128199

129200
const handleClick = (e: React.MouseEvent) => {
@@ -154,6 +225,20 @@ const TreeNodeContent: React.FC<{
154225

155226
const isSelected = selectedPath === node.path;
156227

228+
// Compute read status for files and directories
229+
let isFullyRead = false;
230+
let isUnknownState = false;
231+
232+
if (node.isDirectory) {
233+
const dirStatus = computeDirectoryReadStatus(node, getFileReadStatus);
234+
isFullyRead = dirStatus === "fully-read";
235+
isUnknownState = dirStatus === "unknown";
236+
} else if (getFileReadStatus) {
237+
const readStatus = getFileReadStatus(node.path);
238+
isFullyRead = readStatus ? readStatus.read === readStatus.total && readStatus.total > 0 : false;
239+
isUnknownState = readStatus === null;
240+
}
241+
157242
return (
158243
<>
159244
<TreeNode depth={depth} isSelected={isSelected} onClick={handleClick}>
@@ -162,7 +247,9 @@ const TreeNodeContent: React.FC<{
162247
<ToggleIcon isOpen={isOpen} data-toggle onClick={handleToggleClick}>
163248
164249
</ToggleIcon>
165-
<DirectoryName>{node.name || "/"}</DirectoryName>
250+
<DirectoryName isFullyRead={isFullyRead} isUnknownState={isUnknownState}>
251+
{node.name || "/"}
252+
</DirectoryName>
166253
{node.totalStats &&
167254
(node.totalStats.additions > 0 || node.totalStats.deletions > 0) && (
168255
<DirectoryStats isOpen={isOpen}>
@@ -184,7 +271,9 @@ const TreeNodeContent: React.FC<{
184271
) : (
185272
<>
186273
<span style={{ width: "12px" }} />
187-
<FileName>{node.name}</FileName>
274+
<FileName isFullyRead={isFullyRead} isUnknownState={isUnknownState}>
275+
{node.name}
276+
</FileName>
188277
{node.stats && (
189278
<Stats>
190279
{node.stats.additions > 0 && <Additions>+{node.stats.additions}</Additions>}
@@ -205,6 +294,7 @@ const TreeNodeContent: React.FC<{
205294
selectedPath={selectedPath}
206295
onSelectFile={onSelectFile}
207296
commonPrefix={commonPrefix}
297+
getFileReadStatus={getFileReadStatus}
208298
/>
209299
))}
210300
</>
@@ -217,6 +307,7 @@ interface FileTreeExternalProps {
217307
onSelectFile: (path: string | null) => void;
218308
isLoading?: boolean;
219309
commonPrefix?: string | null;
310+
getFileReadStatus?: (filePath: string) => { total: number; read: number } | null;
220311
}
221312

222313
export const FileTree: React.FC<FileTreeExternalProps> = ({
@@ -225,6 +316,7 @@ export const FileTree: React.FC<FileTreeExternalProps> = ({
225316
onSelectFile,
226317
isLoading = false,
227318
commonPrefix = null,
319+
getFileReadStatus,
228320
}) => {
229321
// Find the node at the common prefix path to start rendering from
230322
const startNode = React.useMemo(() => {
@@ -262,6 +354,7 @@ export const FileTree: React.FC<FileTreeExternalProps> = ({
262354
selectedPath={selectedPath}
263355
onSelectFile={onSelectFile}
264356
commonPrefix={commonPrefix}
357+
getFileReadStatus={getFileReadStatus}
265358
/>
266359
))
267360
) : (

src/components/RightSidebar/CodeReview/HunkViewer.tsx

Lines changed: 69 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,13 @@ import { SelectableDiffRenderer } from "../../shared/DiffRenderer";
1010
interface HunkViewerProps {
1111
hunk: DiffHunk;
1212
isSelected?: boolean;
13+
isRead?: boolean;
1314
onClick?: () => void;
15+
onToggleRead?: () => void;
1416
onReviewNote?: (note: string) => void;
1517
}
1618

17-
const HunkContainer = styled.div<{ isSelected: boolean }>`
19+
const HunkContainer = styled.div<{ isSelected: boolean; isRead: boolean }>`
1820
background: #1e1e1e;
1921
border: 1px solid #3e3e42;
2022
border-radius: 4px;
@@ -24,18 +26,21 @@ const HunkContainer = styled.div<{ isSelected: boolean }>`
2426
transition: all 0.2s ease;
2527
2628
${(props) =>
27-
props.isSelected &&
29+
props.isRead &&
2830
`
29-
border-color: #007acc;
30-
box-shadow: 0 0 0 1px #007acc;
31+
border-color: var(--color-read);
3132
`}
3233
33-
&:hover {
34-
border-color: #007acc;
35-
}
34+
${(props) =>
35+
props.isSelected &&
36+
`
37+
border-color: var(--color-review-accent);
38+
box-shadow: 0 0 0 1px var(--color-review-accent);
39+
`}
3640
`;
3741

3842
const HunkHeader = styled.div`
43+
/* Keep grayscale to avoid clashing with green/red LoC indicators */
3944
background: #252526;
4045
padding: 8px 12px;
4146
border-bottom: 1px solid #3e3e42;
@@ -44,6 +49,7 @@ const HunkHeader = styled.div`
4449
align-items: center;
4550
font-family: var(--font-monospace);
4651
font-size: 12px;
52+
gap: 8px;
4753
`;
4854

4955
const FilePath = styled.div`
@@ -124,19 +130,64 @@ const RenameInfo = styled.div`
124130
}
125131
`;
126132

133+
const ReadIndicator = styled.span`
134+
display: inline-flex;
135+
align-items: center;
136+
color: var(--color-read);
137+
font-size: 14px;
138+
margin-right: 4px;
139+
`;
140+
141+
const ToggleReadButton = styled.button`
142+
background: transparent;
143+
border: 1px solid #3e3e42;
144+
border-radius: 3px;
145+
padding: 2px 6px;
146+
color: #888;
147+
font-size: 11px;
148+
cursor: pointer;
149+
transition: all 0.2s ease;
150+
display: flex;
151+
align-items: center;
152+
gap: 4px;
153+
154+
&:hover {
155+
background: rgba(255, 255, 255, 0.05);
156+
border-color: var(--color-read);
157+
color: var(--color-read);
158+
}
159+
160+
&:active {
161+
transform: scale(0.95);
162+
}
163+
`;
164+
127165
export const HunkViewer: React.FC<HunkViewerProps> = ({
128166
hunk,
129167
isSelected,
168+
isRead = false,
130169
onClick,
170+
onToggleRead,
131171
onReviewNote,
132172
}) => {
133-
const [isExpanded, setIsExpanded] = useState(true);
173+
// Collapse by default if marked as read
174+
const [isExpanded, setIsExpanded] = useState(!isRead);
175+
176+
// Auto-collapse when marked as read, auto-expand when unmarked
177+
React.useEffect(() => {
178+
setIsExpanded(!isRead);
179+
}, [isRead]);
134180

135181
const handleToggleExpand = (e: React.MouseEvent) => {
136182
e.stopPropagation();
137183
setIsExpanded(!isExpanded);
138184
};
139185

186+
const handleToggleRead = (e: React.MouseEvent) => {
187+
e.stopPropagation();
188+
onToggleRead?.();
189+
};
190+
140191
// Parse diff lines
141192
const diffLines = hunk.content.split("\n").filter((line) => line.length > 0);
142193
const lineCount = diffLines.length;
@@ -153,9 +204,11 @@ export const HunkViewer: React.FC<HunkViewerProps> = ({
153204
return (
154205
<HunkContainer
155206
isSelected={isSelected ?? false}
207+
isRead={isRead}
156208
onClick={onClick}
157209
role="button"
158210
tabIndex={0}
211+
data-hunk-id={hunk.id}
159212
onKeyDown={(e) => {
160213
if (e.key === "Enter" || e.key === " ") {
161214
e.preventDefault();
@@ -164,6 +217,7 @@ export const HunkViewer: React.FC<HunkViewerProps> = ({
164217
}}
165218
>
166219
<HunkHeader>
220+
{isRead && <ReadIndicator title="Marked as read"></ReadIndicator>}
167221
<FilePath>{hunk.filePath}</FilePath>
168222
<LineInfo>
169223
{!isPureRename && (
@@ -175,6 +229,11 @@ export const HunkViewer: React.FC<HunkViewerProps> = ({
175229
<LineCount>
176230
({lineCount} {lineCount === 1 ? "line" : "lines"})
177231
</LineCount>
232+
{onToggleRead && (
233+
<ToggleReadButton onClick={handleToggleRead} title="Mark as read (m)">
234+
{isRead ? "○" : "◉"}
235+
</ToggleReadButton>
236+
)}
178237
</LineInfo>
179238
</HunkHeader>
180239

@@ -190,11 +249,12 @@ export const HunkViewer: React.FC<HunkViewerProps> = ({
190249
oldStart={hunk.oldStart}
191250
newStart={hunk.newStart}
192251
onReviewNote={onReviewNote}
252+
onLineClick={onClick}
193253
/>
194254
</HunkContent>
195255
) : (
196256
<CollapsedIndicator onClick={handleToggleExpand}>
197-
Click to expand ({lineCount} lines)
257+
{isRead && "Hunk marked as read. "}Click to expand ({lineCount} lines)
198258
</CollapsedIndicator>
199259
)}
200260

src/components/RightSidebar/CodeReview/ReviewControls.tsx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,10 @@ export const ReviewControls: React.FC<ReviewControlsProps> = ({
166166
onFiltersChange({ ...filters, includeUncommitted: e.target.checked });
167167
};
168168

169+
const handleShowReadToggle = (e: React.ChangeEvent<HTMLInputElement>) => {
170+
onFiltersChange({ ...filters, showReadHunks: e.target.checked });
171+
};
172+
169173
const handleSetDefault = () => {
170174
setDefaultBase(filters.diffBase);
171175
};
@@ -210,6 +214,11 @@ export const ReviewControls: React.FC<ReviewControlsProps> = ({
210214
Uncommitted
211215
</CheckboxLabel>
212216

217+
<CheckboxLabel>
218+
<input type="checkbox" checked={filters.showReadHunks} onChange={handleShowReadToggle} />
219+
Show read
220+
</CheckboxLabel>
221+
213222
<UntrackedStatus
214223
workspaceId={workspaceId}
215224
workspacePath={workspacePath}
@@ -220,7 +229,7 @@ export const ReviewControls: React.FC<ReviewControlsProps> = ({
220229
<Separator />
221230

222231
<StatBadge>
223-
{stats.total} {stats.total === 1 ? "hunk" : "hunks"}
232+
{stats.read} read / {stats.total} total
224233
</StatBadge>
225234
</ControlsContainer>
226235
);

0 commit comments

Comments
 (0)