Fix large-PDF tree and XREF rendering performance

frontend/src/components/TreePanel.tsx

@@ -2,13 +2,26 @@
  * @file PDF object-tree panel. Renders the hierarchical document structure
  * using react-arborist with lazy child loading and cross-reference navigation.
  */
-import { useState, useRef, useEffect, useCallback } from 'react';
+import { useState, useRef, useEffect, useCallback, useMemo, createContext, useContext } from 'react';
 import { useLatest } from '../hooks/useLatest';
 import { Tree, type TreeApi, type NodeRendererProps } from 'react-arborist';
 import { BookOpen, FolderTree, FileText, FileCode, Image as ImageIcon, Type, type LucideIcon } from 'lucide-react';
 import { GetChildren, GetAncestorPath } from '../../bindings/unidoc-pdf-debugger/internal/pdfservice/pdfservice.js';
 import { useAppState, useAppDispatch, type TreeNode } from '../hooks/useDocumentState';
 
+/**
+ * Per-row transient state (which node is mid-load, which is flashing) delivered
+ * to NodeRenderer via context rather than props. react-arborist memoizes its
+ * rows and rebuilds its entire O(N) node model whenever any Tree prop identity
+ * changes; passing these through the render-prop would force that rebuild on
+ * every load/flash toggle. A context change instead re-renders only the row
+ * consumers, leaving the Tree props (and the model) untouched.
+ */
+const RowStateContext = createContext<{ loadingNodeId: string | null; flashNodeId: string | null }>({
+  loadingNodeId: null,
+  flashNodeId: null,
+});
+
 /**
  * Map a backend iconHint to a lucide-react icon component. Returns null for
  * "default" and unknown hints so untyped scalars/arrays render without
@@ -116,6 +129,18 @@ function buildInitialData(rootNode: TreeNode, rootChildren: TreeNode[] | null):
   return [root];
 }
 
+/** Map a backend node id to its react-arborist display id by walking the tree. */
+function findDisplayId(data: TreeNodeData[], backendId: string): string | undefined {
+  for (const n of data) {
+    if (n.backendId === backendId) return n.id;
+    if (n.children) {
+      const found = findDisplayId(n.children, backendId);
+      if (found) return found;
+    }
+  }
+  return undefined;
+}
+
 /** Recursively find a node by ID and replace its children immutably. */
 function updateNodeChildren(
   data: TreeNodeData[],
@@ -134,12 +159,14 @@ function updateNodeChildren(
 }
 
 /** Custom row renderer for tree nodes. Handles selection, flash, and error styling. */
-function NodeRenderer({ node, style, dragHandle, isLoading, flashNodeIdRef }: NodeRendererProps<TreeNodeData> & { isLoading?: boolean; flashNodeIdRef?: React.RefObject<string | null> }) {
+function NodeRenderer({ node, style, dragHandle }: NodeRendererProps<TreeNodeData>) {
+  const { loadingNodeId, flashNodeId } = useContext(RowStateContext);
   const data = node.data;
   const isError = data.error !== '';
   const isSelected = node.isSelected;
   const isInternal = node.isInternal;
-  const isFlashing = data.id === flashNodeIdRef?.current;
+  const isLoading = data.id === loadingNodeId;
+  const isFlashing = data.id === flashNodeId;
 
   // Hide rawKey when the inline objectRef suffix is rendered. PDFBox-style
   // rows show "Pages [2 0 R]" rather than "Pages /Pages [2 0 R]" -- the
@@ -257,8 +284,7 @@ export function TreePanel() {
   const selectedNodeIdRef = useLatest(selectedNodeId);
   const treeDataRef = useLatest(treeData);
   const treeRef = useRef<TreeApi<TreeNodeData> | undefined>(undefined);
-  const [, setFlashNodeId] = useState<string | null>(null);
-  const flashNodeIdRef = useRef<string | null>(null);
+  const [flashNodeId, setFlashNodeId] = useState<string | null>(null);
 
   // Container sizing for react-arborist
   const containerRef = useRef<HTMLDivElement>(null);
@@ -433,11 +459,9 @@ export function TreePanel() {
           payload: { nodeId: targetNode.backendId, label: targetNode.name, rawKey: targetNode.rawKey, iconHint: targetNode.iconHint },
         });
 
-        // Flash effect
-        flashNodeIdRef.current = targetNode.id;
+        // Flash effect (delivered to rows via RowStateContext)
         setFlashNodeId(targetNode.id);
         setTimeout(() => {
-          flashNodeIdRef.current = null;
           setFlashNodeId(null);
         }, 100);
 
@@ -530,18 +554,25 @@ export function TreePanel() {
     // selectedNodeIdRef is a stable useLatest ref; listed to satisfy exhaustive-deps.
   }, [dispatch, selectedNodeIdRef]);
 
-  // Map backendId to display id for react-arborist's selection prop
-  function findDisplayId(data: TreeNodeData[], backendId: string): string | undefined {
-    for (const n of data) {
-      if (n.backendId === backendId) return n.id;
-      if (n.children) {
-        const found = findDisplayId(n.children, backendId);
-        if (found) return found;
-      }
-    }
-    return undefined;
-  }
-  const selectionDisplayId = selectedNodeId ? findDisplayId(treeData, selectedNodeId) : undefined;
+  // Memoized so the `selection` prop keeps a stable identity across renders that
+  // change neither the selection nor the tree. react-arborist rebuilds its entire
+  // O(N) node model whenever ANY prop identity in treeProps changes (see its
+  // provider's `api.update(treeProps)` memo), so an unstable prop forces a full
+  // rebuild on every render -- pathological on large trees.
+  const selectionDisplayId = useMemo(
+    () => (selectedNodeId ? findDisplayId(treeData, selectedNodeId) : undefined),
+    [selectedNodeId, treeData],
+  );
+
+  // Fully stable child render-prop: an inline arrow (or one keyed on loadingNodeId)
+  // is a new function whenever it changes, and react-arborist rebuilds its whole
+  // O(N) model on any Tree prop identity change. Per-row loading/flash now flow
+  // through RowStateContext instead, so this never needs to change.
+  const renderNode = useCallback(
+    (props: NodeRendererProps<TreeNodeData>) => <NodeRenderer {...props} />,
+    [],
+  );
+  const rowState = useMemo(() => ({ loadingNodeId, flashNodeId }), [loadingNodeId, flashNodeId]);
 
   return (
     <div className="h-full flex flex-col" data-testid="tree-panel">
@@ -550,6 +581,7 @@ export function TreePanel() {
       </div>
       <div ref={containerRef} className="h-full w-full relative flex-1 min-h-0">
         {dimensions.width > 0 && dimensions.height > 0 && treeData.length > 0 && (
+          <RowStateContext.Provider value={rowState}>
           <Tree<TreeNodeData>
             key={activeTabId ?? ''}
             ref={treeRef}
@@ -569,10 +601,9 @@ export function TreePanel() {
             width={dimensions.width}
             height={dimensions.height}
           >
-            {(props: NodeRendererProps<TreeNodeData>) => (
-              <NodeRenderer {...props} isLoading={loadingNodeId === props.node.id} flashNodeIdRef={flashNodeIdRef} />
-            )}
+            {renderNode}
           </Tree>
+          </RowStateContext.Provider>
         )}
         {navError && (
           <div

frontend/src/components/XRefTableView.test.tsx

@@ -422,19 +422,34 @@ describe('9.11-UNIT-014: error rendering', () => {
 });
 
 // ---------------------------------------------------------------------------
-// 9.11-UNIT-015 [P1] AC#2: eager fetch on mount so the parent can populate
-// the "XREF (N)" tab label without waiting for first tab activation. The
-// `active` prop no longer gates the fetch (revised after 9-12).
+// 9.11-UNIT-015 [P1]: the fetch is DEFERRED until the XREF tab is first
+// activated. The payload can be very large (a 129k-entry PDF serializes ~12 MB);
+// because the pane is force-mounted, an unconditional fetch would JSON.parse
+// ~12 MB and render all rows on the main thread on EVERY document open, freezing
+// the UI while the user is still on the Object tree. active=false must NOT fetch;
+// activation triggers a single fetch. (Supersedes the pre-perf "eager fetch on
+// mount regardless of active" behavior.)
 // ---------------------------------------------------------------------------
 
-describe('9.11-UNIT-015: eager fetch on mount', () => {
+describe('9.11-UNIT-015: fetch deferred until activation', () => {
   beforeEach(() => {
     vi.clearAllMocks();
     mockGetXRefTable.mockResolvedValue(xrefBasic);
   });
 
-  test('active=false still fetches eagerly', async () => {
+  test('active=false does not fetch', () => {
     render(<XRefTableView tabId="tab-1" active={false} onNavigate={vi.fn()} onLoaded={vi.fn()} />);
+    // render() flushes mount effects under act; with active=false the fetch
+    // effect exits synchronously, so no fetch is scheduled -- assert immediately.
+    expect(mockGetXRefTable).not.toHaveBeenCalled();
+  });
+
+  test('activation after an inactive mount triggers a single fetch', async () => {
+    const { rerender } = render(
+      <XRefTableView tabId="tab-1" active={false} onNavigate={vi.fn()} onLoaded={vi.fn()} />,
+    );
+    expect(mockGetXRefTable).not.toHaveBeenCalled();
+    rerender(<XRefTableView tabId="tab-1" active={true} onNavigate={vi.fn()} onLoaded={vi.fn()} />);
     await waitFor(() => {
       expect(mockGetXRefTable).toHaveBeenCalledWith('tab-1');
     });
@@ -448,6 +463,131 @@ describe('9.11-UNIT-015: eager fetch on mount', () => {
     });
     expect(mockGetXRefTable).toHaveBeenCalledTimes(1);
   });
+
+  test('switching documents while XREF is inactive does NOT eagerly fetch; re-activating fetches the new doc', async () => {
+    // Distinct data per document so re-activation can be asserted by rendered rows.
+    mockGetXRefTable.mockImplementation((id: string) =>
+      Promise.resolve(id === 'tab-2' ? xrefSingleInUse : xrefBasic),
+    );
+    const { rerender } = render(
+      <XRefTableView tabId="tab-1" active={true} onNavigate={vi.fn()} onLoaded={vi.fn()} />,
+    );
+    await screen.findByTestId('xref-row-objnum-1'); // tab-1 fetched + rendered
+    expect(mockGetXRefTable).toHaveBeenCalledTimes(1);
+
+    // Switch documents with the XREF tab INACTIVE (parent lands on Object). A
+    // stale activation must NOT eagerly fetch the new, unopened document.
+    rerender(<XRefTableView tabId="tab-2" active={false} onNavigate={vi.fn()} onLoaded={vi.fn()} />);
+    await new Promise((r) => setTimeout(r, 20));
+    expect(mockGetXRefTable).not.toHaveBeenCalledWith('tab-2');
+    expect(mockGetXRefTable).toHaveBeenCalledTimes(1);
+
+    // Re-activating XREF on the new document fetches and renders it.
+    rerender(<XRefTableView tabId="tab-2" active={true} onNavigate={vi.fn()} onLoaded={vi.fn()} />);
+    expect(await screen.findByTestId('xref-row-objnum-7')).toBeInTheDocument();
+    expect(mockGetXRefTable).toHaveBeenCalledWith('tab-2');
+  });
+
+  test('returning to a previously-opened document on the Object tab does NOT eagerly re-fetch', async () => {
+    mockGetXRefTable.mockResolvedValue(xrefBasic);
+    // Doc A: open XREF (latches A).
+    const { rerender } = render(
+      <XRefTableView tabId="tab-1" active={true} onNavigate={vi.fn()} onLoaded={vi.fn()} />,
+    );
+    await screen.findByTestId('xref-row-objnum-1');
+    expect(mockGetXRefTable).toHaveBeenCalledTimes(1);
+    // Switch to doc B on the Object tab (inactive).
+    rerender(<XRefTableView tabId="tab-2" active={false} onNavigate={vi.fn()} onLoaded={vi.fn()} />);
+    // Switch BACK to doc A, still on the Object tab (inactive). The latch must
+    // have been cleared on the first switch, so returning to A must NOT re-fetch.
+    rerender(<XRefTableView tabId="tab-1" active={false} onNavigate={vi.fn()} onLoaded={vi.fn()} />);
+    await new Promise((r) => setTimeout(r, 20));
+    expect(mockGetXRefTable).toHaveBeenCalledTimes(1); // still just the original doc-A fetch
+  });
+});
+
+// ---------------------------------------------------------------------------
+// 9.11-UNIT-017 [P1] Perf: the row list is viewport-virtualized -- a large xref
+// (a 750-page PDF can carry ~129k entries) must render only a bounded window of
+// rows, not one <tr> per entry, or the main thread freezes on render.
+// ---------------------------------------------------------------------------
+
+describe('9.11-UNIT-017: row list is virtualized', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  test('a large table renders only a bounded window of rows', async () => {
+    const big: XRefTableFixture = {
+      tabId: 'tab-1',
+      entries: Array.from({ length: 3000 }, (_, i) => ({
+        objNum: i + 1,
+        gen: 0,
+        status: 'in-use' as const,
+        offset: (i + 1) * 16,
+        hostObjStm: 0,
+        nodeID: `obj:0:${i + 1}`,
+      })),
+    };
+    mockGetXRefTable.mockResolvedValue(big);
+    const { container } = render(
+      <XRefTableView tabId="tab-1" active={true} onNavigate={vi.fn()} onLoaded={vi.fn()} />,
+    );
+    await waitFor(() => {
+      expect(container.querySelectorAll('[data-testid^="xref-row-objnum-"]').length).toBeGreaterThan(0);
+    });
+    // Only a small window commits to the DOM (jsdom viewport fallback ~= 36
+    // rows), never all 3000. A spacer <tr> reserves the off-window scroll height.
+    const rendered = container.querySelectorAll('[data-testid^="xref-row-objnum-"]').length;
+    expect(rendered).toBeGreaterThan(0);
+    expect(rendered).toBeLessThan(200);
+    expect(container.querySelectorAll('tr[aria-hidden="true"]').length).toBeGreaterThan(0);
+  });
+});
+
+// ---------------------------------------------------------------------------
+// 9.11-UNIT-018 [P1] AC#4 + virtualization: ArrowDown past the rendered window
+// scrolls the next row into view and focuses it. DOM-sibling focus cannot work
+// here because off-window rows are unmounted; the handler walks the index.
+// ---------------------------------------------------------------------------
+
+describe('9.11-UNIT-018: keyboard nav crosses the virtualization window', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    const big: XRefTableFixture = {
+      tabId: 'tab-1',
+      entries: Array.from({ length: 3000 }, (_, i) => ({
+        objNum: i + 1,
+        gen: 0,
+        status: 'in-use' as const,
+        offset: (i + 1) * 16,
+        hostObjStm: 0,
+        nodeID: `obj:0:${i + 1}`,
+      })),
+    };
+    mockGetXRefTable.mockResolvedValue(big);
+  });
+
+  test('ArrowDown from the last in-window row focuses the next (initially unrendered) row', async () => {
+    render(<XRefTableView tabId="tab-1" active={true} onNavigate={vi.fn()} onLoaded={vi.fn()} />);
+    // Row 36 is at the edge of the initial ~36-row window; row 37 is not rendered.
+    const row36 = await screen.findByTestId('xref-row-36');
+    expect(screen.queryByTestId('xref-row-37')).toBeNull();
+    row36.focus();
+    fireEvent.keyDown(row36, { key: 'ArrowDown' });
+    // Row 37 must scroll in AND receive focus.
+    const row37 = await screen.findByTestId('xref-row-37');
+    await waitFor(() => expect(document.activeElement).toBe(row37));
+  });
+
+  test('ArrowUp at the top row does not wrap', async () => {
+    render(<XRefTableView tabId="tab-1" active={true} onNavigate={vi.fn()} onLoaded={vi.fn()} />);
+    const row1 = await screen.findByTestId('xref-row-1');
+    row1.focus();
+    fireEvent.keyDown(row1, { key: 'ArrowUp' });
+    // No wrap: focus stays on row 1.
+    expect(document.activeElement).toBe(row1);
+  });
 });
 
 // ---------------------------------------------------------------------------