perf(review): debounce search highlights and O(1) active match swap (#407)

* perf(review): debounce search highlights and O(1) active match swap Split the single search highlight effect into two paths: 1. Full rebuild (debounced 100ms) — fires on query/matches/file/style changes, collapsing rapid keystrokes into one DOM update instead of one per character typed. 2. Active swap (instant) — fires when stepping between matches (Enter/Shift+Enter), changing two elements' background color via querySelector instead of tearing down and rebuilding every <mark> in the shadow DOM. Also removes augmentedDiff from the effect deps (was triggering unnecessary rebuilds on context expansion) and exports clearSearchHighlights for the immediate-clear path on Escape. Closes #405 For provenance purposes, this commit was AI assisted. * perf(review): debounce search query and pre-index searchable lines The search input was laggy on large PRs because findReviewSearchMatches re-parsed every file's patch and scanned all lines synchronously on every keystroke, blocking React from re-rendering the input value. Two complementary fixes: 1. Pre-index: Build SearchableLine[] once when files change (useMemo), with pre-lowercased normalizedText, so matching skips repeated patch parsing and per-line toLowerCase() calls. 2. Debounced query: searchQuery updates instantly (input responsive), debouncedSearchQuery trails by 200ms and drives all expensive work. During the pending window, sidebar shows "Searching…", diff clears stale highlights, match count hides, and stepping is disabled. Also: refactors findReviewSearchMatches to delegate to the new findMatchesInIndex so there is a single matching implementation, removes the now-redundant 100ms DOM debounce in DiffViewer (query debounce handles it upstream), trims query in highlightQuery to prevent whitespace-only edits from breaking snippet highlighting, and restores augmentedDiff in the highlight effect deps. For provenance purposes, this commit was AI assisted.
2026-09-14 14:17:26 +08:00 · 2026-03-26 22:24:50 -07:00
parent 232cae265e
commit bcddced148
6 changed files with 127 additions and 49 deletions
@@ -147,6 +147,8 @@ const ReviewApp: React.FC = () => {

  const {
    searchQuery,
+    debouncedSearchQuery,
+    isSearchPending,
    isSearchOpen,
    activeSearchMatchId,
    activeSearchMatch,
@@ -336,7 +338,7 @@ const ReviewApp: React.FC = () => {
      }

      // Enter/F3 to step through search matches
-      if ((e.key === 'Enter' || e.key === 'F3') && searchMatches.length > 0 && !isTypingTarget(e.target)) {
+      if ((e.key === 'Enter' || e.key === 'F3') && searchMatches.length > 0 && !isSearchPending && !isTypingTarget(e.target)) {
        e.preventDefault();
        stepSearchMatch(e.shiftKey ? -1 : 1);
        return;
@@ -362,7 +364,7 @@ const ReviewApp: React.FC = () => {
    window.addEventListener('keydown', handleKeyDown);
    return () => window.removeEventListener('keydown', handleKeyDown);
  // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [showExportModal, showDestinationMenu, searchQuery, searchMatches, openSearch, stepSearchMatch, clearSearch, files, gitContext?.diffOptions]);
+  }, [showExportModal, showDestinationMenu, searchQuery, searchMatches, isSearchPending, openSearch, stepSearchMatch, clearSearch, files, gitContext?.diffOptions]);

  // Get annotations for active file
  const activeFileAnnotations = useMemo(() => {
@@ -1337,6 +1339,7 @@ const ReviewApp: React.FC = () => {
                currentBranch={gitContext?.currentBranch}
                stagedFiles={stagedFiles}
                searchQuery={searchQuery}
+                isSearchPending={isSearchPending}
                searchInputRef={searchInputRef}
                onSearchChange={handleSearchInputChange}
                onSearchClear={clearSearch}
@@ -1389,7 +1392,7 @@ const ReviewApp: React.FC = () => {
                onStage={() => stageFile(activeFile.path)}
                canStage={canStageFiles}
                stageError={stageError}
-                searchQuery={searchQuery}
+                searchQuery={isSearchPending ? '' : debouncedSearchQuery}
                searchMatches={activeFileSearchMatches}
                activeSearchMatchId={activeSearchMatchId}
                activeSearchMatch={activeSearchMatch?.filePath === activeFile.path ? activeSearchMatch : null}
@@ -16,8 +16,10 @@ import { SuggestionModal } from './SuggestionModal';
 import { type ReviewSearchMatch } from '../utils/reviewSearch';
 import {
  applySearchHighlights,
+  clearSearchHighlights,
  getSearchRoots,
  retryScrollToSearchMatch,
+  swapActiveSearchHighlight,
 } from '../utils/reviewSearchHighlight';

 interface DiffViewerProps {
@@ -204,17 +206,33 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
    return () => clearTimeout(timeoutId);
  }, [selectedAnnotationId]);

-  // Apply search highlights to diff lines (including inside shadow DOM)
+  // Apply search highlights to diff lines (including inside shadow DOM).
+  // The query is already debounced upstream (useReviewSearch), so this runs synchronously.
+  // activeSearchMatchId is NOT in deps — the swap effect handles that with O(1) updates.
  useEffect(() => {
    if (!containerRef.current) return;

-    const frameId = requestAnimationFrame(() => {
-      const roots = getSearchRoots(containerRef.current!);
-      roots.forEach(root => applySearchHighlights(root, searchQuery, searchMatches, activeSearchMatchId));
-    });
+    const query = searchQuery;
+    const matches = searchMatches;

-    return () => cancelAnimationFrame(frameId);
-  }, [searchQuery, searchMatches, activeSearchMatchId, filePath, diffStyle, augmentedDiff]);
+    if (!query.trim() || matches.length === 0) {
+      const roots = getSearchRoots(containerRef.current);
+      roots.forEach(root => clearSearchHighlights(root));
+      return;
+    }
+
+    const roots = getSearchRoots(containerRef.current);
+    roots.forEach(root =>
+      applySearchHighlights(root, query, matches, activeSearchMatchId)
+    );
+  }, [searchQuery, searchMatches, filePath, diffStyle, augmentedDiff]);
+
+  // Swap active search highlight instantly when stepping between matches.
+  // This avoids a full rebuild just to change two elements' background color.
+  useEffect(() => {
+    if (!containerRef.current) return;
+    swapActiveSearchHighlight(containerRef.current, activeSearchMatchId);
+  }, [activeSearchMatchId]);

  // Scroll to active search match (with retry for lazy-rendered content)
  useEffect(() => {
@@ -27,6 +27,7 @@ interface FileTreeProps {
  currentBranch?: string;
  stagedFiles?: Set<string>;
  searchQuery?: string;
+  isSearchPending?: boolean;
  searchInputRef?: React.RefObject<HTMLInputElement | null>;
  onSearchChange?: (value: string) => void;
  onSearchClear?: () => void;
@@ -58,6 +59,7 @@ export const FileTree: React.FC<FileTreeProps> = ({
  currentBranch,
  stagedFiles,
  searchQuery = '',
+  isSearchPending,
  searchInputRef,
  onSearchChange,
  onSearchClear,
@@ -166,7 +168,7 @@ export const FileTree: React.FC<FileTreeProps> = ({
                  e.preventDefault();
                  return;
                }
-                if (e.key === 'Enter' && searchMatches.length > 0) {
+                if (e.key === 'Enter' && searchMatches.length > 0 && !isSearchPending) {
                  e.preventDefault();
                  onStepSearchMatch?.(e.shiftKey ? -1 : 1);
                }
@@ -184,7 +186,7 @@ export const FileTree: React.FC<FileTreeProps> = ({
            />
            {searchQuery && (
              <div className="absolute right-2 top-1/2 -translate-y-1/2 flex items-center gap-1">
-                {searchQuery.trim() && (
+                {searchQuery.trim() && !isSearchPending && (
                  <span className="text-[10px] text-muted-foreground/40 tabular-nums">
                    {searchMatches.length}
                  </span>
@@ -332,7 +334,11 @@ export const FileTree: React.FC<FileTreeProps> = ({
      {/* File tree or search results */}
      <div className="flex-1 overflow-y-auto p-2">
        {searchQuery.trim() ? (
-          searchGroups.length > 0 ? (
+          isSearchPending ? (
+            <div className="py-6 text-center text-xs text-muted-foreground/50">
+              Searching…
+            </div>
+          ) : searchGroups.length > 0 ? (
            searchGroups.map((group) => (
              <SearchFileGroup
                key={group.filePath}
@@ -393,8 +399,9 @@ export const FileTree: React.FC<FileTreeProps> = ({
 // --- Search result components ---

 function highlightQuery(text: string, query: string) {
-  if (!query.trim()) return text;
-  const regex = new RegExp(`(${query.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')})`, 'gi');
+  const trimmed = query.trim();
+  if (!trimmed) return text;
+  const regex = new RegExp(`(${trimmed.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')})`, 'gi');
  const parts = text.split(regex);
  // split with a capturing group puts matches at odd indices (1, 3, 5...)
  return parts.map((part, i) =>
@@ -1,7 +1,8 @@
 import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
 import {
-  findReviewSearchMatches,
  groupReviewSearchMatches,
+  buildSearchIndex,
+  findMatchesInIndex,
  type ReviewSearchMatch,
  type ReviewSearchableDiffFile,
 } from '../utils/reviewSearch';
@@ -33,14 +34,29 @@ export function useReviewSearch({
  clearPendingSelection,
 }: UseReviewSearchOptions) {
  const [searchQuery, setSearchQuery] = useState('');
+  const [debouncedSearchQuery, setDebouncedSearchQuery] = useState('');
  const [isSearchOpen, setIsSearchOpen] = useState(false);
  const [activeSearchMatchId, setActiveSearchMatchId] = useState(null as string | null);

  const searchInputRef = useRef(null as HTMLInputElement | null);

+  useEffect(() => {
+    if (!searchQuery.trim()) {
+      setDebouncedSearchQuery('');
+      return;
+    }
+    const timeoutId = setTimeout(() => setDebouncedSearchQuery(searchQuery), 200);
+    return () => clearTimeout(timeoutId);
+  }, [searchQuery]);
+
+  const isSearchPending = searchQuery.trim() !== ''
+    && searchQuery.trim().toLowerCase() !== debouncedSearchQuery.trim().toLowerCase();
+
+  const searchIndex = useMemo(() => buildSearchIndex(files), [files]);
+
  const searchMatches = useMemo(() => {
-    return findReviewSearchMatches(files, searchQuery);
-  }, [files, searchQuery]);
+    return findMatchesInIndex(searchIndex, debouncedSearchQuery);
+  }, [searchIndex, debouncedSearchQuery]);

  const searchGroups = useMemo(() => {
    return groupReviewSearchMatches(files, searchMatches);
@@ -68,6 +84,7 @@ export function useReviewSearch({

  const clearSearch = useCallback(() => {
    setSearchQuery('');
+    setDebouncedSearchQuery('');
    setActiveSearchMatchId(null);
    setIsSearchOpen(false);
  }, []);
@@ -95,7 +112,7 @@ export function useReviewSearch({
  }, [searchMatches, activeSearchMatchId]);

  useEffect(() => {
-    if (!searchQuery.trim() || searchMatches.length === 0) {
+    if (!debouncedSearchQuery.trim() || searchMatches.length === 0) {
      setActiveSearchMatchId(null);
      return;
    }
@@ -106,7 +123,7 @@ export function useReviewSearch({
    if (!stillExists) {
      setActiveSearchMatchId(searchMatches[0].id);
    }
-  }, [searchQuery, searchMatches, activeSearchMatchId]);
+  }, [debouncedSearchQuery, searchMatches, activeSearchMatchId]);

  useEffect(() => {
    if (!activeSearchMatch) return;
@@ -120,6 +137,8 @@ export function useReviewSearch({

  return {
    searchQuery,
+    debouncedSearchQuery,
+    isSearchPending,
    isSearchOpen,
    activeSearchMatchId,
    activeSearchMatch,
@@ -42,12 +42,13 @@ export interface ReviewSearchFileGroup {
  matches: ReviewSearchMatch[];
 }

-interface SearchableLine {
+export interface SearchableLine {
  filePath: string;
  side: ReviewSearchSide;
  lineNumber: number;
  altLineNumber?: number;
  text: string;
+  normalizedText: string;
 }

 function isDiffMetadataLine(line: string): boolean {
@@ -84,6 +85,7 @@ function buildSearchableLinesForPatch(file: ReviewSearchableDiffFile): Searchabl
        lineNumber: newLine,
        altLineNumber: oldLine,
        text,
+        normalizedText: text.toLowerCase(),
      });
      continue;
    }
@@ -95,6 +97,7 @@ function buildSearchableLinesForPatch(file: ReviewSearchableDiffFile): Searchabl
        side: 'deletion',
        lineNumber: oldLine,
        text,
+        normalizedText: text.toLowerCase(),
      });
      continue;
    }
@@ -106,6 +109,7 @@ function buildSearchableLinesForPatch(file: ReviewSearchableDiffFile): Searchabl
        side: 'addition',
        lineNumber: newLine,
        text,
+        normalizedText: text.toLowerCase(),
      });
    }
  }
@@ -113,6 +117,10 @@ function buildSearchableLinesForPatch(file: ReviewSearchableDiffFile): Searchabl
  return searchableLines;
 }

+export function buildSearchIndex(files: ReviewSearchableDiffFile[]): SearchableLine[] {
+  return files.flatMap(file => buildSearchableLinesForPatch(file));
+}
+
 function buildSnippet(text: string, start: number, end: number): string {
  const snippetStart = Math.max(0, start - SNIPPET_CONTEXT);
  const snippetEnd = Math.min(text.length, end + SNIPPET_CONTEXT);
@@ -121,47 +129,46 @@ function buildSnippet(text: string, start: number, end: number): string {
  return `${prefix}${text.slice(snippetStart, snippetEnd)}${suffix}`;
 }

-export function findReviewSearchMatches(files: ReviewSearchableDiffFile[], query: string): ReviewSearchMatch[] {
+export function findMatchesInIndex(index: SearchableLine[], query: string): ReviewSearchMatch[] {
  const trimmedQuery = query.trim();
  if (!trimmedQuery) return [];

  const normalizedQuery = trimmedQuery.toLowerCase();
  const matches: ReviewSearchMatch[] = [];

-  files.forEach((file) => {
-    const searchableLines = buildSearchableLinesForPatch(file);
+  index.forEach((line) => {
+    let fromIndex = 0;
+    let matchCountForLine = 0;

-    searchableLines.forEach((line) => {
-      const normalizedText = line.text.toLowerCase();
-      let fromIndex = 0;
-      let matchCountForLine = 0;
+    while (fromIndex <= line.normalizedText.length) {
+      const matchStart = line.normalizedText.indexOf(normalizedQuery, fromIndex);
+      if (matchStart === -1) break;

-      while (fromIndex <= normalizedText.length) {
-        const matchStart = normalizedText.indexOf(normalizedQuery, fromIndex);
-        if (matchStart === -1) break;
+      const matchEnd = matchStart + normalizedQuery.length;
+      matches.push({
+        id: `${line.filePath}:${line.side}:${line.lineNumber}:${matchStart}:${matchCountForLine}`,
+        filePath: line.filePath,
+        side: line.side,
+        lineNumber: line.lineNumber,
+        altLineNumber: line.altLineNumber,
+        text: line.text,
+        matchStart,
+        matchEnd,
+        snippet: buildSnippet(line.text, matchStart, matchEnd),
+      });

-        const matchEnd = matchStart + normalizedQuery.length;
-        matches.push({
-          id: `${line.filePath}:${line.side}:${line.lineNumber}:${matchStart}:${matchCountForLine}`,
-          filePath: line.filePath,
-          side: line.side,
-          lineNumber: line.lineNumber,
-          altLineNumber: line.altLineNumber,
-          text: line.text,
-          matchStart,
-          matchEnd,
-          snippet: buildSnippet(line.text, matchStart, matchEnd),
-        });
-
-        matchCountForLine += 1;
-        fromIndex = matchStart + Math.max(1, normalizedQuery.length);
-      }
-    });
+      matchCountForLine += 1;
+      fromIndex = matchStart + Math.max(1, normalizedQuery.length);
+    }
  });

  return matches;
 }

+export function findReviewSearchMatches(files: ReviewSearchableDiffFile[], query: string): ReviewSearchMatch[] {
+  return findMatchesInIndex(buildSearchIndex(files), query);
+}
+
 export function groupReviewSearchMatches(
  files: ReviewSearchableDiffFile[],
  matches: ReviewSearchMatch[],
@@ -28,7 +28,7 @@ export function getSearchRoots(root: ParentNode): ParentNode[] {
  return roots;
 }

-function clearSearchHighlights(root: ParentNode) {
+export function clearSearchHighlights(root: ParentNode) {
  const marks = root.querySelectorAll('mark[data-review-search-match]');
  marks.forEach((mark) => {
    const parent = mark.parentNode;
@@ -66,6 +66,11 @@ function decorateSearchMatch(mark: HTMLElement, isActive: boolean) {
  mark.style.borderRadius = '3px';
  mark.style.padding = '0 1px';
  mark.style.boxShadow = isActive ? ACTIVE_MATCH_RING : PASSIVE_MATCH_RING;
+  if (isActive) {
+    mark.dataset.reviewSearchActive = '';
+  } else {
+    delete mark.dataset.reviewSearchActive;
+  }
 }

 function lineKey(match: ReviewSearchMatch): string {
@@ -150,6 +155,25 @@ export function applySearchHighlights(
  });
 }

+export function swapActiveSearchHighlight(
+  container: HTMLElement,
+  newActiveId: string | null,
+): void {
+  const roots = getSearchRoots(container);
+  for (const root of roots) {
+    const prev = root.querySelector('mark[data-review-search-active]') as HTMLElement | null;
+    if (prev) {
+      decorateSearchMatch(prev, false);
+    }
+    if (newActiveId) {
+      const next = root.querySelector(`mark[data-review-search-match="${newActiveId}"]`) as HTMLElement | null;
+      if (next) {
+        decorateSearchMatch(next, true);
+      }
+    }
+  }
+}
+
 export function scrollToSearchMatch(root: ParentNode, match: ReviewSearchMatch): boolean {
  const lineEl = root.querySelector(getLineSelector(match)) as HTMLElement | null;
  if (!lineEl) return false;