🤖 Review tab search highlighting and performance #343
Merged
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 sleek search input above hunks - Search matches filenames and hunk content (case-insensitive) - Real-time filtering with 300ms debounce - Updated empty state to show search-specific messages Generated with `cmux`
Extract filter logic to dedicated utility for clarity and testability: - filterByReadState() - filters by read/unread status - filterBySearch() - searches filenames and content - applyFrontendFilters() - composes filters in order Benefits: - Explicit separation between git-level and frontend filters - Better documentation of architecture and rationale - More testable (filters can be unit tested independently) - Simpler to extend with new filter types - Preserves truncation handling (path filter stays git-level) Net: -11 lines in ReviewPanel, +65 lines in new utility Generated with `cmux`
- Add FOCUS_REVIEW_SEARCH keybind definition in keybinds.ts - Wire up global keyboard shortcut in ReviewPanel - Display keybind hint in search input placeholder - Update keybinds.ts documentation note about UI discoverability Keybind shown in placeholder so docs update not needed. Generated with `cmux`
- Add styled RegexButton component with active/inactive states - Update SearchContainer to flexbox layout with gap - Add isRegexSearch state to ReviewPanel - Extend filterBySearch to support regex mode with error handling - Button shows '.⋆' and highlights when active - Invalid regex patterns return empty results gracefully - Tooltip indicates current search mode Generated with `cmux`
Storage changes: - Add getReviewSearchInputKey, getReviewSearchRegexKey, getReviewSearchMatchCaseKey - Add to PERSISTENT_WORKSPACE_KEY_FUNCTIONS for fork/delete handling - Search input, regex toggle, and match case now persist per workspace Filter changes: - Extend filterBySearch to support matchCase parameter - Case-sensitive regex: omit 'i' flag when matchCase is true - Case-sensitive substring: use includes() instead of toLowerCase() UI changes: - Rename RegexButton to SearchButton for reusability - Add Match Case button showing 'Aa' with active/inactive states - Both buttons persist per-workspace and copy on fork - Clear tooltips for both modes Generated with `cmux`
…ltips - Unified storage: Single usePersistedState for all search state (input, regex, matchCase) - Visual cohesion: Removed gaps between input and buttons, connected borders - Greyscale active state: Changed from blue (#007acc) to subtle grey (#3a3a3a) - Tooltips: Replaced HTML title with Tooltip component for consistency - Cleanup: Removed three separate storage keys in favor of one unified key
- Add line-height: 1.4 to both input and buttons for uniform height - Remove conditional border-left logic that was hiding the border between input and first button - All buttons now have consistent borders creating a seamless connected appearance
Key improvements: - Introduced SearchBar wrapper that owns border/radius/focus state - Children (input/buttons) are now borderless with only internal dividers - Removed fragile border connection logic (border-right: none, conditional border-left) - Removed isLast prop - radius now handled by parent's overflow: hidden - Focus state lifted to parent with :focus-within pseudo-class - Hover state also on parent, cleaner interaction model - Buttons use transparent background instead of duplicating parent color - Simpler, more maintainable with clear separation of concerns
- Created highlightSearchTerms utility to post-process Shiki HTML - Uses DOMParser to safely inject <mark> tags around matches - Preserves syntax highlighting while adding subtle gold overlay - Supports both substring and regex search with case sensitivity - Threads searchConfig through ReviewPanel → HunkViewer → SelectableDiffRenderer - Only highlights when search term is present (no performance impact when idle) - Gracefully handles invalid regex patterns
Phase 1 - React optimizations: - Memoize searchConfig object in ReviewPanel to prevent cascading re-renders - Memoize highlighted line data in SelectableDiffRenderer - Eliminates 50+ unnecessary re-renders per keystroke when typing Phase 2 - DOM parsing optimizations: - Add LRU cache (5000 entries, 2MB) for highlighted HTML results - Cache compiled regex patterns to avoid recompilation - Reuse single DOMParser instance instead of creating new one per line - Use CRC32 checksums for cache keys (same pattern as tokenizer) Expected performance improvement: - Typing in search: ~200-500ms → <16ms - Scrolling with search active: Smooth (memoized results) - DOM parsing only happens once per unique line + search config combination
Replace Map with LRUCache for compiled regex patterns to match the pattern used for highlighted HTML cache. Set max 100 entries which is plenty for typical search usage.
Switch from caching (html + config → highlighted HTML) to (html → parsed DOM). Benefits: - DOM parsing only happens once per unique HTML line - Cache persists across different search terms - DOM cloning is cheaper than re-parsing - Allows cache reuse when user modifies search query Trade-off: We clone and re-highlight on each render, but cloning + highlighting is still faster than parsing + highlighting, and the cache hit rate is much higher.
- Exported escapeHtml() from highlightDiffChunk.ts for reuse - Added useMemo in HunkViewer to highlight filePath with same logic as diff content - Uses dangerouslySetInnerHTML to render highlighted path with <mark> tags - Consistent gold highlight styling (rgba(255, 215, 0, 0.3)) - Zero additional complexity: reuses existing highlightSearchMatches() utility
- Moved mark.search-highlight styles from LineContent to App.tsx globalStyles - Ensures consistent highlight appearance across diff content and file paths - Eliminates duplicate CSS definitions - Global styling pattern matches other app-wide styles (fonts, colors, scrollbars)
- Active buttons now show cool blue text color (#4db8ff) - Added subtle inset blue border/shadow (rgba(77, 184, 255, 0.4)) when active - Changed active background to blue-tinted dark (#2a3a4a) for cohesion - Fixed height alignment: buttons and input now fill container completely - Added display: flex and height: 100% to ensure consistent vertical fill - Active state is now much more visually apparent while maintaining clean aesthetic
![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".
- Lazy-load DOMParser to avoid instantiation in non-browser environments - Prevents 'ReferenceError: DOMParser is not defined' in unit tests - getParser() function creates instance on first use - No performance impact: parser still cached after first call
- Use workingDoc.createElement/createTextNode instead of global document - Prevents WrongDocumentError when replacing nodes across different Documents - Fragment, mark elements, and text nodes now belong to the same Document - Fixes search highlighting not appearing due to replaceChild throwing
- Changed if statement to ??= operator in getParser() - Cleaner and more idiomatic TypeScript - Satisfies @typescript-eslint/prefer-nullish-coalescing
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.
Summary
Comprehensive enhancement to Review tab search with visual highlighting, performance optimizations, and polished UI. Search now highlights matches in both diff content and file paths with <16ms response time.
Key Features
🔍 Visual Search Highlighting
rgba(255, 215, 0, 0.3)) wraps matches in<mark>tags⚡ Performance Optimizations
🎨 UI Polish
usePersistedStatefor input/regex/matchCase)Architecture
State Management:
ReviewSearchStatetype replaces three separate hooksData Flow:
Caching Strategy:
CRC32(html)only (config-independent)${term}:${useRegex}:${matchCase}Testing
Manual testing confirms:
Generated with
cmux