🤖 Add Code Review tab with interactive diff viewing and review notes #315
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Renamed the right sidebar component for clearer naming: - ChatMetaSidebar → RightSidebar - Updated all imports, component names, and internal references - Updated localStorage keys for consistency - Preserved git history using git mv Generated with cmux
Implemented a complete code review interface in the right sidebar: Core Features: - Parse git diffs into reviewable hunks with stable IDs - Accept/reject hunks with optional notes - Filter by review status (all/accepted/rejected/unreviewed) - Toggle visibility of reviewed hunks - Keyboard navigation (j/k, a/r for accept/reject) - Stale review detection and cleanup Components: - HunkViewer: Displays individual diff hunks with syntax highlighting - ReviewActions: Action buttons for accept/reject with note input - ReviewFilters: Status filters and statistics display - ReviewPanel: Main container coordinating all review functionality Storage: - Reviews persisted to localStorage per workspace - useReviewState hook for managing review state - Statistics calculated on-demand Integration: - Added "Review" tab to RightSidebar (alongside Costs and Tools) - Fetches diff from workspace using git diff HEAD - Auto-collapses long hunks (>20 lines) Generated with cmux
Changed ReviewPanel to use existing workspace.executeBash IPC call instead of attempting to run git commands directly in renderer process. - Removed getWorkspaceDiff from diffParser (was using Node child_process) - ReviewPanel now calls window.api.workspace.executeBash with 'git diff HEAD' - Fixed to use result.data.output (not stdout) per BashToolResult type Fixes: Module "child_process" has been externalized for browser compatibility Generated with cmux
Users can now choose what to diff against instead of always using HEAD: Options: - HEAD (uncommitted changes) - default - Staged changes (--staged flag) - main branch - origin/main - Custom ref (with text input) Implementation: - Added diffBase field to ReviewFilters type - New dropdown selector in ReviewFilters component - Custom ref input appears when "Custom ref..." selected - ReviewPanel rebuilds git diff command based on selected base - Reloads diff automatically when base changes This allows reviewing: - Uncommitted changes vs last commit - Staged changes only - Feature branch vs main/trunk - Any arbitrary git ref comparison Generated with cmux
Fixed issue where diff base selector was hidden when there were no changes, making it impossible to change the base. Changes: - Moved ReviewFilters outside of conditional rendering - Filters now always visible at top of panel - Loading state, empty state, and hunk list are now mutually exclusive - Updated empty state message to mention trying different base - Removed unused hunkId prop from ReviewActionsProps interface User can now change diff base even when current base has no changes, enabling exploration of different branches/refs. Generated with cmux
Review tab now gets more horizontal and vertical space for better code review UX: Width changes: - Costs/Tools tabs: 300px (unchanged) - Review tab: 600px (2x width) - Animated transition: 0.2s ease (existing transition) Vertical changes: - Review tab: No padding (15px removed top/bottom = +30px) - Costs/Tools tabs: Keep 15px padding (unchanged) Implementation: - Added 'wide' prop to SidebarContainer (conditional 600px width) - Added 'noPadding' prop to TabContent (conditional padding removal) - Both props triggered when selectedTab === 'review' Result: Review tab smoothly expands to 600px width and uses full vertical space, while other tabs maintain their original compact layout. Generated with cmux
Review tab now stays expanded and never collapses to vertical bar: Logic: - When Review tab is active: Always expanded (never collapses) - When Costs/Tools tabs are active: Normal hysteresis behavior - Collapse at chatAreaWidth <= 800px - Expand at chatAreaWidth >= 1100px - Dead zone 800-1100px maintains state This prevents janky collapse/expand cycles when switching between tabs. Review tab genuinely needs space for code diffs, so forcing it to stay expanded provides better UX than collapsing mid-review. Trade-off: Review at 600px + ChatArea min 750px = 1350px minimum window. On smaller screens, horizontal scroll appears - acceptable for code review. Generated with cmux
Fixed issue where selecting 'main' as diff base showed 'No changes found'. Previous behavior: - Selecting 'main' ran: git diff main - This compared working directory to main branch - If on main with no uncommitted changes: showed nothing New behavior: - HEAD: git diff HEAD (uncommitted changes) - --staged: git diff --staged (staged changes only) - main/origin/main/etc: git diff main...HEAD (branch comparison) The three-dot syntax (base...HEAD) shows changes since the common ancestor, which is what you want when reviewing a feature branch against main. Example: If you're on 'feature' branch and select 'main' as base, you now see all commits on 'feature' that aren't on 'main', regardless of working directory state. Generated with cmux
Added visible error state to show git command failures in the UI instead of silently hiding them in console. Changes: - Added error state variable - ErrorState styled component (red background, monospace) - Display error above loading/empty states - Clear error on each new diff attempt - Show full git error message to user This makes it visible when git commands fail (e.g., invalid ref, no common ancestor, etc) instead of just showing 'No changes found'. Generated with cmux
Added command display to 'No changes found' state for debugging clarity. Changes: - Store last executed git command in state - Display command in monospace box below empty state message - Example: "Command: git diff main...HEAD" This helps users understand exactly what git command ran and debug why they're not seeing expected changes (e.g., wrong ref name, branch not diverged yet, etc). Generated with cmux
Improved command display UX for easier copy-paste: Before: - Single line: "Command: git diff main...HEAD" - Hard to select just the command After: - Label: "COMMAND" (uppercase, small, gray) - Value: "git diff main...HEAD" (monospace, user-select: all) - Easily triple-click to select entire command This makes it simple to copy the exact command for manual testing in terminal. Generated with cmux
Restructured command display with label outside the box: Before: ┌─────────────────────────┐ │ COMMAND │ │ git diff main...HEAD │ └─────────────────────────┘ After: COMMAND ┌─────────────────────────┐ │ git diff main...HEAD │ └─────────────────────────┘ Changes: - Label now sits above the command box (not inside) - Command box is clean with just the selectable text - Added cursor: text for better UX - Max-width: 600px on section for readability Much cleaner visual hierarchy - label is clearly a label, box contains the actual command value. Generated with cmux
Show expandable diagnostic section when no changes found: - Command executed - Output size (bytes) - Files parsed count - Hunks extracted count Helps debug parsing issues without console logs
Use overflow_policy to set limits based on use case: - tmpfile (AI agent): 16KB/300 lines (conservative for LLM context) - truncate (IPC/UI): 1MB/10K lines (generous for UI features) Add truncated field to BashToolResult: - success: true + truncated metadata for partial results - UI gets structured truncation info instead of parsing errors ReviewPanel improvements: - Uses truncated field to detect partial diffs - Shows warning banner when diff exceeds limits - Displays all collected hunks (up to 10K lines worth) This fixes large branch diffs (main...HEAD) that were showing empty due to 16KB limit. Now handles most real-world diffs gracefully.
Tests use real git repository instead of mocked diffs: - Single and multiple file modifications - New file additions and deletions - Branch comparisons with three-dot syntax - Empty diffs - Stable hunk ID generation - Large diffs with multiple hunks All tests create actual git repos, make real changes, and verify parsing works correctly with real git diff output.
Features: - Parse git diff --numstat to get file stats (additions/deletions) - Build hierarchical file tree from flat paths - Display collapsible tree with +/- stats per file - Click file to filter hunks to that file only - File tree appears on right side (300px width) - Auto-expand first 2 directory levels - Clear filter button when file is selected Layout changes: - Split review pane into side-by-side: hunks (flex) + file tree (300px) - File tree has border-left separator - Parallel fetch of diff and numstat for performance Filtering: - Hunks filtered by filePath when file selected - Empty state shows helpful message based on filter - File selection is toggle (click again to clear)
Change width from fixed 600px to dynamic: - Up to 1200px max - Or calc(100vw - 400px) to leave room for chat - Whichever is smaller Gives Review tab much more space for side-by-side hunks + file tree.
Add min-height: 0 to all flex containers to enable proper scrolling: - PanelContainer, ContentContainer, HunksSection, HunkList - FileTreeSection with flex-shrink: 0 - TreeContainer with flex: 1 Remove padding from TabContent when Review tab is active. This fixes the nested flex layout issue where content was cut off. Now hunks and file tree scroll independently as intended.
Implement encapsulated, scroll-safe resize functionality for Review tab: **New hook: useResizableSidebar** - Encapsulates all drag logic (mousedown/move/up) - localStorage persistence per workspace - Min/max width constraints - Only enabled when Review tab active - Clean separation from scroll containers **Integration**: - ResizeHandle component between ChatArea and RightSidebar - 4px draggable border (blue on hover/active) - RightSidebar accepts optional width prop in Review mode - Tab state tracked in AIView, communicated via onTabChange callback **Key design**: - No DOM wrapping - preserves existing scroll container hierarchy - ChatArea min-width reduced to 400px (from 750px) for flexibility - Width persists across sessions via localStorage - Drag state prevents text selection during resize Scroll functionality completely preserved - no interference with: - Auto-scroll during streaming - Jump-to-bottom indicator - Manual scrolling - Scroll bars _Generated with `cmux`_
Document the drag-resize feature with detailed inline comments: **useResizableSidebar hook**: - Full JSDoc header explaining design principles and usage - Inline comments for all interfaces and major logic sections - Explains drag math (deltaX calculation from right edge) - Documents event listener lifecycle and cleanup - Notes on localStorage persistence and error handling **AIView integration**: - Comments on tab tracking and callback flow - ResizeHandle component purpose and visibility logic - Hook instantiation with inline param explanations - Render section comments showing data flow **RightSidebar width system**: - Width priority documentation (collapsed > custom > wide > default) - JSDoc for SidebarContainer explaining each width mode - Comments on onTabChange callback purpose - Inline comments clarifying conditional logic Goal: Future maintainers (human or AI) can understand the resize system without tracing through code. Comments explain *why* not just *what*. _Generated with `cmux`_
Clean up RightSidebar and improve tab navigation:
**Removed**:
- Tools tab (unused functionality)
- ToolsTab import and component
- Per-workspace tab persistence
**Added**:
- Keyboard shortcuts: Cmd+1 (Costs), Cmd+2 (Review)
- Ctrl on Linux/Windows per platform detection
- Tooltip hints showing keybinds on tab hover
- Global tab preference via usePersistedState
**Changes**:
- TabType: "costs" | "tools" | "review" → "costs" | "review"
- Tab state: per-workspace → global ("right-sidebar-tab")
- Keyboard handler in useEffect with cleanup
- formatKeybind() for cross-platform display
**UX improvements**:
- Discoverability: hover tabs to see keyboard shortcuts
- Persistence: tab selection survives workspace switches
- Consistency: tab shortcuts match app-wide keybind patterns
_Generated with `cmux`_
Fix type errors by adding the missing keybind definitions to KEYBINDS object. _Generated with `cmux`_
Replace native title tooltips with styled Tooltip component for better visibility and consistency with rest of app. Changes: - Import TooltipWrapper and Tooltip components - Wrap each TabButton in TooltipWrapper - Show formatted keybind in styled tooltip on hover - Position: bottom, align: center for clear display Tooltips now use app's styled tooltip system (dark bg, white text, arrow) instead of browser default, making keyboard shortcuts more discoverable. _Generated with `cmux`_
The styled Tooltip component inadvertently changed tab layout/styling. Revert to simple title attribute to preserve original appearance while still showing keyboard shortcuts on hover. _Generated with `cmux`_
Fix tab layout by making TooltipWrapper participate in flex layout: - TabBar: Apply flex: 1 to all direct children (TooltipWrapper) - TabButton: Change from flex: 1 to width: 100% (fills wrapper) - Wrap each tab in TooltipWrapper with styled Tooltip Result: Tabs maintain equal width distribution while showing styled tooltips on hover. Best of both worlds - proper layout + visible hints. _Generated with `cmux`_
- Extract shared diff rendering logic into DiffRenderer component - Reused by both FileEditToolCall and HunkViewer for consistency - Centralizes colors, line numbers, indicators in one place - Integrate review status badges into action buttons - Remove separate ReviewBadge from HunkViewer header - Show ACCEPTED/REJECTED badges directly in buttons when active - Buttons now highlight when status is active - Implement responsive layout with flexbox - File tree moves above hunks on narrow viewports (<800px) - Use CSS order property and media queries - Better use of vertical space on smaller screens - Fix linting issues - Replace .match() with RegExp.exec() - Remove unused catch variables _Generated with `cmux`_
**ReviewControls Component** - Create new one-line consolidated control bar - Replace ReviewFilters with more compact layout - Convert stat badges into clickable filter buttons - Add active states to show current filter selection - Reduce vertical space usage **Directory-Level Stats** - Add totalStats to FileTreeNode interface - Calculate and display LoC summaries for directories (grey text) - Helps users understand change scope at directory level **Persistence & Performance** - Persist diff base selection per workspace with usePersistedState - Fix re-render bug: remove selectedHunkId from loadDiff dependencies - Clicking a hunk no longer triggers unnecessary diff reload - Only reload diff when workspace, path, or diffBase changes **Responsive Layout Fixes** - FileTree now uses minimal height on narrow viewports - Set flex: 0 0 auto on narrow viewports (no grow/shrink) - Hunks section remains scrollable at all viewport sizes - File tree moves above hunks cleanly without height issues **Visual Polish** - Add text-overflow: ellipsis to hunk/file headers (no line breaks) - Make truncation warning more succinct (11px, less padding) - Consistent spacing and sizing throughout controls _Generated with `cmux`_
**VerticalTokenMeter Always Visible** - Show VerticalTokenMeter on non-Costs tabs (Review, etc.) - Fixes flash bug when switching from Review to Costs - Context always apparent with token usage visible **Folder Filtering** - Allow clicking folders to filter hunks from that folder - Toggle icon (▶) still expands/collapses folder - Clicking folder name/stats filters to show only that folder's hunks - Preserves caret toggle behavior with data-toggle attribute **Server-Side Path Filtering** - Re-run git diff with path filter when selecting file/folder - Improves performance for large diffs - Fixes truncation warning showing when file selected - Truncation warning only shows when viewing all files - Keep file tree visible when filtering to maintain navigation **Responsive Layout Fix** - FileTreeSection limited to 40vh max height on narrow viewports - Minimum height of 150px to show some files - HunksSection uses flex: 1 with min-height: 0 for proper scrolling - Fixed issue where hunks disappeared on narrow viewports - Files Changed section now properly constrained **Code Changes:** - Remove client-side path filtering (now server-side) - Add selectedFilePath to useEffect dependencies - Only reload file tree when not filtering - Clear truncation warning when path filter active _Generated with `cmux`_
**VerticalTokenMeter Positioning**
- Position on left edge with absolute positioning (z-index: 5)
- Uses VerticalMeterWrapper for consistent placement
- Appears left of drag bar, matching collapsed view position
- No longer appears above ReviewControls
**Independent Loading States**
- Split loading into isLoadingTree and isLoadingHunks
- FileTree loads independently from hunks
- Changing file filter doesn't reload file tree
- FileTree shows "Loading file tree..." message during load
- Hunks show loading only when no data yet
**Diff Highlight Fix**
- Red/green backgrounds now extend to full scrollable width
- Added min-width: 100% to DiffLine
- Added min-width: fit-content to DiffContainer children
- Highlights no longer cut off when scrolling horizontally
**File Filter Persistence**
- Persist selectedFilePath per workspace
- Key: `review-file-filter:${workspaceId}`
- Filter survives workspace switches and page reloads
**Responsive Layout Fix**
- FileTreeSection: Fixed 250px height on narrow viewports
- Uses flex: 0 0 250px for explicit sizing
- HunksSection properly fills remaining space
- Both sections independently scrollable
**Code Organization**
- Remove unused numstatCommand variable
- Remove unused FileTreeProps interface
- FileTree accepts null root with loading state
- Use nullish coalescing (??) over logical or (||)
_Generated with `cmux`_
**Hide Hunk Header** - Remove @@ hunk header from diff display - Header cuts off important file names - Parse line numbers but don't render the header - Cleaner, more focused diff view **Simplify VerticalTokenMeter** - Revert complex absolute positioning attempt - Keep simple: only show in collapsed view (Costs tab) - Remove VerticalTokenMeter from Review tab entirely - Focus should be on code review, not token usage - Eliminates overlay bug and positioning complexity **Fix Diff Highlight Backgrounds** - Move padding from DiffContainer to DiffLine - Padding: 6px 0 on container, 0 8px on lines - min-width: calc(100% - 16px) accounts for padding - Red/green backgrounds now extend full width when scrolling - No more cut-off highlights on long lines **Clean Up Button Styling** - Reduce button padding: 8px 16px → 5px 10px - Smaller font size: 13px → 11px - Smaller gaps: 8px → 6px - Lighter borders when inactive (transparent background) - Reduce status badge size: 9px → 8px font - Reduce note input padding and min-height - More compact, less obtrusive design - Keybind hints smaller and more subtle **Result:** - Cleaner, more professional appearance - Better space utilization - No VerticalTokenMeter overlay issues - Full-width diff backgrounds work correctly - Consistent, compact button styling _Generated with `cmux`_
Features:
1. Include selected lines in review notes with context
- Line numbers + indicators (+/-/space) shown in code block
- Format: ```lineNum indicator content```
- Provides full context for AI to understand review comment
2. Fixed multi-line drag selection
- Preserve original startIndex during drag
- Use sortedStart/sortedEnd for line number calculation
- Drag up or down now works correctly
3. Less obtrusive textarea styling
- Transparent container background (was solid gray)
- Semi-transparent textarea (95% opacity, 98% on focus)
- Yellow-gold borders matching selection highlight
- Smaller, tighter spacing (60px → 50px min-height)
- Smaller buttons (11px → 10px font)
- Reduced padding throughout
Visual result:
- Textarea floats over diff without blocking view
- Yellow theme consistent throughout selection UI
- More compact, less visual weight
Example output:
<review>
Re src/file.ts:87-89
```
87 + const result = foo();
88 + if (!result) {
89 + return null;
```
> Can we deduplicate this?
</review>
_Generated with `cmux`_
Changes: 1. Restored solid background color (#252526) - Was transparent, now matches app theme - Cleaner visual separation from diff content 2. Increased top padding (4px → 10px) - Better breathing room above textarea - More comfortable visual spacing 3. Removed Cancel and Add to Chat buttons - Eliminated wasted horizontal space - Keyboard shortcuts remain: Cmd+Enter to submit, Esc to cancel - Cleaner, more compact UI - All functionality preserved via keyboard 4. Restored solid textarea background (#1e1e1e) - Was semi-transparent, now opaque - Better contrast and readability Removed code: - NoteActions styled component - NoteButton styled component - Button rendering JSX (~20 lines) Result: Much more compact textarea UI with all functionality intact via keyboard shortcuts. _Generated with `cmux`_
Feature: Standard shift-click behavior for multi-line selection - Click first line to start selection - Shift-click another line to extend selection to that line - Works in both directions (up or down) Implementation: - handleMouseDown now accepts shiftKey parameter - If shift is held and selection exists: extend from original start to clicked line - If shift not held: start new selection (existing behavior) - Preserves original startIndex like drag selection - Prevents starting drag mode on shift-click UX improvements: - More natural multi-line selection (matches text editor behavior) - Three ways to select ranges now: 1. Click and drag 2. Click, then shift-click 3. Click, then click to extend (existing behavior) _Generated with `cmux`_
Changed placeholder to: - 'Add a review note to chat' (clearer about destination) - Added 'Shift-click to select range' (documents new feature) - Kept Cmd+Enter and Esc shortcuts Full text: "Add a review note to chat (Shift-click to select range, Cmd+Enter to submit, Esc to cancel)" More descriptive and helps users discover the shift-click feature. _Generated with `cmux`_
Added infrastructure for review notes in tool call diffs: 1. FileEditToolCall now supports review notes - Added onReviewNote optional prop - renderDiff() conditionally uses SelectableDiffRenderer when callback provided - Falls back to read-only DiffRenderer when no callback - Passes filePath to review notes for context 2. Updated component chain to support callback - ToolMessage accepts and passes onReviewNote - MessageRenderer accepts onReviewNote prop - All three FileEdit tool variants wired up Implementation details: - Uses SelectableDiffRenderer for interactive selection - Same UX as Code Review tab: drag, shift-click, keyboard shortcuts - Review notes include selected code with line numbers - Graceful fallback when onReviewNote not provided Note: Not yet wired from AIView - callback will be undefined until MessageRenderer is called with onReviewNote from parent. _Generated with `cmux`_
Removed onReviewNote prop from: - MessageRenderer (not connected from AIView) - ToolMessage (not needed without MessageRenderer) Kept in FileEditToolCall: - onReviewNote prop and conditional SelectableDiffRenderer logic - Infrastructure ready for when we want to wire it through later This commit removes WIP code that isn't currently used. FileEditToolCall still supports review notes via the optional callback, but without the callback wired from parent components, it falls back to read-only DiffRenderer. _Generated with `cmux`_
Removed drag-to-select functionality: - Removed isDragging state - Removed handleMouseEnter (drag extension logic) - Removed handleMouseUp and global mouseup listener - Changed onMouseDown back to onClick - Removed preventDefault() that blocked text selection - Removed user-select: none CSS Users can now: - Select text normally for copy/paste (drag to select text) - Click lines to start review note selection - Shift-click to extend selection to multiple lines - No conflict between text selection and line selection Shift-click remains the only way to select multiple lines for review notes, which is more explicit and avoids ambiguity with text selection. _Generated with `cmux`_
Added subtle refresh icon to left of Base input: - 12px circular arrow SVG icon - Subtle gray color (#888) that lightens on hover - Triggers full diff and file tree reload - Uses refreshTrigger state counter pattern Implementation: - RefreshButton styled component (transparent, minimal) - onRefresh callback prop in ReviewControls - refreshTrigger state in ReviewPanel increments on click - Both diff and file tree useEffects depend on refreshTrigger - Icon only shows when onRefresh callback provided UX: - Positioned before Base label for quick access - Tooltip: "Refresh diff" - Hover feedback with color transition - Clean, unobtrusive design _Generated with `cmux`_
…nsolidate styling
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
ammar-agent
force-pushed
the
code-review
branch
from
55b53d4 to
8e220da
Compare
ammar-agent
force-pushed
the
code-review
branch
from
8e220da to
8f07c1a
Compare
|
Far from perfect, but a good start. |
IPC bash calls explicitly use overflow_policy: "truncate" (10K line limit) to avoid temp file spam from background operations. The test was incorrectly expecting tmpfile behavior (300 line limit). Updated test to verify that 400 lines return successfully without truncation, matching actual IPC behavior.
The Tools tab was removed from the right sidebar when the Code Review feature was added. Updated the review.spec.ts test to use "Code Review" tab instead of the non-existent "Tools" tab.
The right sidebar tab is labeled "Review", not "Code Review".
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds an interactive Code Review tab to the right sidebar with diff viewing and inline review notes.
Key Features
Review Panel
Interactive Diff Selection
File Tree Improvements
Technical Details
Architecture:
ReviewPanel- Main container managing state and git operationsSelectableDiffRenderer- Interactive diff with line selectionReviewControls- Compact control bar with filtersFileTree- Hierarchical file browser with common prefix extractionDisplay Logic Encapsulation:
State Management:
usePersistedStatefor per-workspace settingsreview-diff-base,review-include-dirty,review-file-filterTesting
Recent Polish
This PR includes recent UX improvements:
useLayoutModehookGenerated with
cmux