🤖 Add read-more feature to code review hunks #419
+1,336
−90
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
- Add HunkReadMoreState type to track expansion state (up/down lines) - Add getReviewReadMoreStateKey storage helper for persistence - Create readFileLines utility using sed over existing executeBash IPC - Add expand up/down buttons in HunkViewer with ~30 line increments - Load and display expanded context with proper diff formatting - Store user expansion preferences per hunk in localStorage - Support expanding until beginning of file (disable button when reached) - Display loading states during line fetching
ammar-agent
force-pushed
the
cr-readmore
branch
from
3b690b5 to
91e04f7
Compare
Extract 4 components from 400-line HunkViewer: - HunkHeader (90 lines): File path, stats, read button - ReadMoreButton (40 lines): Reusable expand up/down button - ExpandedDiffContent (40 lines): Renders expanded context - HunkContent (130 lines): Main diff area with read-more logic HunkViewer now 296 lines (down from 400), cleaner separation of concerns.
The read-more feature was reading from working tree with sed, but diffs can be against different bases (HEAD, origin/main, etc.). This caused incorrect content when: - Diffing against old commits - Working tree has changes not in the diff - Line numbers don't align with current file Changes: - Update readFileLines() to accept gitRef parameter - Use 'git show REF:path | sed' instead of 'sed path' - Add getOldFileRef() to determine correct ref from diffBase - Pass diffBase/includeUncommitted through HunkViewer props - Read from appropriate git version (HEAD, diffBase, or working tree) Now reads context lines from the same file version that the diff is using.
Two critical fixes: 1. **Grammar state continuity**: Combine expanded content with original hunk into single diff before highlighting. This ensures multi-line constructs (comments, strings, templates) highlight correctly across boundaries. Before: Three separate highlight passes (broken grammar state) After: Single unified pass through syntax highlighter 2. **Reduce duplication**: Abstract up/down expansion into ExpansionState interface, eliminating 8 duplicate props in HunkContent. Deleted ExpandedDiffContent component (no longer needed).
Critical bug: read-more was only showing 30 lines max, not cumulative. Root cause: calculateUpwardExpansion/calculateDownwardExpansion were calculating incremental ranges with `newExpansion = current + 30` but useEffect was replacing content, not appending. Fix: Changed functions to return full cumulative range: - Upward: Always from `oldStart - currentExpansion` to `oldStart - 1` - Downward: Always from `hunkEnd + 1` to `hunkEnd + currentExpansion` Now clicking 3x shows 30, 60, 90 lines (not 30, 30, 30). Added comprehensive unit tests (7 test cases): - Cumulative expansion behavior - Boundary conditions (line 1, file start) - Single-line and large hunks - Zero-line edge cases All tests passing ✅
Users can now collapse expanded content back down, not just expand. UI Changes: - Show "Read 30 more lines ↑" to expand further - Show "Show 30 fewer lines ↑" to collapse (when expanded) - Buttons appear above/below expanded content appropriately - Collapse reduces by 30 lines at a time (symmetric with expand) Implementation: - Added handleCollapseUp/Down callbacks in HunkViewer - Extended ExpansionState interface with onCollapse + canCollapse - Updated ReadMoreButton to support expand/collapse actions - Button text adapts: "Read more" vs "Show fewer" Tests: - Added 3 collapse test cases to readFileLines.test.ts - Verifies collapse from 60→30→0 works correctly - Handles boundary conditions (can't collapse below 0) - All 10 tests passing ✅ UX Flow: Initial → Expand 30 → Expand 60 → Collapse 30 → Collapse 0
Replace button-based expansion UI with GitHub-style blue arrows: - New ExpanderArrow component shows arrow in gutter - Arrow points up/down based on direction and expansion state - Single click toggles expansion/collapse (30 line increments) - Blue accent color matches GitHub's visual language - Auto-hides when cannot expand and not expanded Key changes: - HunkContent: Use ExpanderArrow instead of ReadMoreButton - HunkViewer: Replace separate expand/collapse handlers with toggle handlers - ExpansionState: Changed from expand/collapse callbacks to single toggle Behavior: - Upward arrow: ▲ (expand up) → ▼ (collapse) - Downward arrow: ▼ (expand down) → ▲ (collapse) - Loading state shows 'Loading...' text - Maintains all existing persistence and state management
Position arrow in gutter aligned with line numbers: - Arrow now appears in line number column (not separate row) - Matches diff line structure: indicator (·) + line number (arrow) + content - Removed extraneous text labels, just show arrow icon - Proper alignment with existing diff gutter Fix collapse visibility: - Always show arrow when expanded (allows collapsing) - Previously hid arrow when fully expanded upward - Now logic: show if expanded OR can expand (not AND) Visual improvements: - Subtle dot (·) in indicator column - Arrow centered in line number column - Minimal hover highlight - Loading shows '...' in line number position
Arrow alignment: - ExpanderArrow now part of SelectableDiffRenderer grid structure - Matches exact column layout: indicator (·) + line number (arrow) + content - Rendered via expanderTop/expanderBottom props - Perfect visual alignment with diff lines Hunk combining infrastructure: - New combineHunks.ts with combineOverlappingHunks() - Detects when expanded hunks overlap or are adjacent (<= 3 line gap) - Returns CombinedHunk[] with sourceHunks tracking - Preserves max expansion state when combining - Stateless presentation logic - pure function - 9 unit tests covering all scenarios Next step: Integrate combining into ReviewPanel to apply before rendering
Presentation layer now combines overlapping hunks stateless: - ReviewPanel loads readMoreStateMap to track expansion - combineOverlappingHunks() called on filtered hunks - Renders CombinedHunk[] instead of individual hunks - Combined hunk is 'read' if all source hunks are read - Toggling read state applies to all source hunks Benefits: - No duplicate context when hunks overlap after expansion - Cleaner UI when expanding shows same code twice - Seamless UX - combined hunks look like single hunks - Stateless logic - pure function based on current state Example: Hunk A (lines 10-15) expanded down 60 lines overlaps with Hunk B (lines 40-45). They render as one combined hunk. Marking as read marks both A and B.
Replace toggle behavior with dual-arrow system: - Show both expand AND collapse arrows when expanded - Expand arrow: solid blue, always adds 30 lines - Collapse arrow: muted (opacity 0.5), removes 30 lines - Click expand multiple times to keep expanding (30→60→90...) - Click collapse to step back down (90→60→30→0) Visual improvements: - BOF marker: Shows 'Beginning of file' at line 1 - EOF marker: Shows 'End of file' when expansion hits end - EOF detection: content.length < requested expansion - Arrows properly aligned in line number gutter Key changes: - ExpanderArrow now takes `mode: 'expand' | 'collapse'` - HunkViewer has 4 handlers: expandUp/collapseUp/expandDown/collapseDown - HunkContent renders both arrows conditionally based on state - New EOFMarker component for end-of-file indicator Behavior: - Collapsed: Show only expand arrow - Expanded: Show both expand (if room) and collapse - At BOF: Show BOF marker, hide upward expand - At EOF: Show EOF marker, hide downward expand This matches GitHub's UX where you can keep expanding without mode switching between expand and collapse.
Bug: combineOverlappingHunks was combining hunks across file boundaries, causing hunks from one file (e.g., EOFMarker.tsx) to show incorrect line counts (954 lines instead of 26). Root cause: Only checked line number overlap, not file path matching. Fix: - hunksOverlap() now checks filePath before checking line ranges - Sort hunks by filePath first, then line number - Added test case to prevent regression Test: 'does not combine hunks from different files' - Creates hunks from file1.ts and file2.ts - Verifies they stay separate even with overlapping line ranges - All 20 tests pass This ensures combined hunks only contain content from a single file.
Better EOF detection after user clicks expand: - Detect empty/no content: hunk was already at EOF - Detect partial content: got fewer lines than requested - Use optional chain for cleaner code Behavior: - User clicks expand down on hunk at EOF - readFileLines returns null or empty content - EOF marker appears immediately after click - No expensive probing on mount (performance) This is the best we can do without file length metadata. Users will see EOF after first expansion attempt.
Show marker text in the content area of collapse arrows: - BOF: Shows 'Beginning of file' next to collapse ↓ arrow - EOF: Shows 'End of file' next to collapse ↑ arrow - Saves 2 rows per file boundary (one line instead of two) Before: [ BOF ] Beginning of file [ ↓ ] Collapse up After: [ ↓ ] Beginning of file Changes: - ExpanderArrow accepts optional markerText prop - Marker text styled: italic, opacity 40% - Removed separate EOFMarker component - Same visual info, 50% less vertical space All 20 tests passing.
Better visual styling for BOF/EOF markers: - Use text-muted color (matches grey from old standalone markers) - Add bullet separator (•) for visual distinction - Flex layout with gap for proper spacing - Italic text maintains marker appearance Visual improvements: - Grey muted color instead of accent blue with opacity - Bullet point separates arrow from text - Better readability and visual hierarchy - Matches the original standalone marker aesthetics Example: [ ↓ ] • Beginning of file All 20 tests passing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Adds read-more buttons to code review hunks that allow expanding context above and below the diff. Uses sed over the existing executeBash IPC to fetch ~30 lines at a time, with state persisted per hunk.
Changes
HunkReadMoreStateto track{ up: int, down: int }expansion per hunkgetReviewReadMoreStateKey()for workspace-scoped persistencereadFileLines.tswith sed-based line reading and expansion calculationsTesting
Test manually:
Generated with
cmux