Virtualize the steps panel (study browser) #984

JackWilb · 2025-12-11T15:38:53Z

Does this PR close any open issues?

Closes #778

Give a longer description of what this PR addresses and why it's needed

This PR virtualizes the steps panel. The first commit modifies the steps panel to render iteratively and the second adds in virtualization using @tanstack/virtual. Since things are handled iteratively, it makes it pretty easy to virtualize, since we know how long the list is and how large each element is.

Provide pictures/videos of the behavior before and after these changes (optional)

In study

Analysis

Are there any additional TODOs before this PR is ready to go?

TODOs:

Update relevant documentation
Add in blocks that were removed from the sequence
Testing on larger sequences (10k working just fine)
Fix analysis render
See if there are any performance improvements I can make (memoizing each row based on href?)
Add an image to the PR

github-actions · 2026-01-08T04:33:32Z

A preview of 03886d0 is uploaded and can be seen here:

✨ https://revisit.dev/study/PR984 ✨

Changes may take a few minutes to propagate.

JackWilb · 2026-01-09T16:19:21Z

Imported library sequences don't indicate that they're imported or which library/sequence they refer to

Copilot

Pull request overview

This PR virtualizes the steps panel (study browser) using @tanstack/react-virtual to improve performance when rendering large sequences. The implementation involves a complete rewrite of the StepsPanel component to pre-compute a flat tree structure and render only visible items.

Key changes:

Complete rewrite of StepsPanel.tsx to use virtualization with fixed-height rows
Enhanced Latin square generation logic to pre-calculate usage counts and avoid mid-sequence refills
Updated component interfaces and type definitions to make skip field required on Sequence type

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 18 comments.

Show a summary per file

File	Description
yarn.lock	Added @tanstack/react-virtual v3.13.13 and updated @trrack/core from beta to stable
package.json	Added @tanstack/react-virtual dependency and updated @trrack/core version
tests/test-reviewer-mode.spec.ts	Updated test to reflect UI changes (tab renamed from "All Trials View" to "Browse Components")
src/utils/handleRandomSequences.tsx	Enhanced Latin square generation with path usage counting, optimized component indexing, and added interruptions field initialization
src/utils/getSequenceFlatMap.ts	Updated addPathToComponentBlock to always initialize skip field as empty array
src/store/types.ts	Changed skip field from optional to required in Sequence interface
src/components/interface/StepsPanel.tsx	Complete rewrite to implement virtualization with flat tree structure, collapse/expand functionality, and excluded components display
src/components/interface/AppAside.tsx	Updated to use new StepsPanel API with simplified props and flexbox layout for virtualization
src/analysis/individualStudy/stats/StatsView.tsx	Updated to use new StepsPanel API

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-09T16:26:18Z

src/utils/handleRandomSequences.tsx

+function generateLatinSquare(config: StudyConfig, path: string) {
+  const pathArr = path.split('-');
+
+  let locationInSequence: Partial<ComponentBlock> | Partial<DynamicBlock> | string = {};
+  pathArr.forEach((p) => {
+    if (p === 'root') {
+      locationInSequence = config.sequence;
+    } else {
+      if (isDynamicBlock(locationInSequence as StudyConfig['sequence'])) {
+        return;
+      }
+      locationInSequence = (locationInSequence as ComponentBlock).components[+p];
+    }
+  });
+
+  const options = (locationInSequence as ComponentBlock).components.map((c: unknown, i: number) => (typeof c === 'string' ? c : `_componentBlock${i}`));
+  shuffle(options);
+  const newSquare: string[][] = latinSquare<string>(options, true);
+  return newSquare;
+}


The function generateLatinSquare is defined at line 23 but was previously defined later in the file (around line 227). Moving it to the top is good for code organization since it's called by the initialization logic. However, this function references 'config' parameter but does not use it anywhere in the function body after generating the Latin square. The config parameter appears to be unnecessary.

Copilot · 2026-01-09T16:26:18Z

src/utils/handleRandomSequences.tsx

+        // Fallback: shouldn't happen, but handle it
+        const index = order.components.findIndex((c) => isEqual(c, curr));
+        computedComponents[i] = _componentBlockToSequence(curr, latinSquareObject, `${path}-${index}`, config) as unknown as ComponentBlock;


The optimization using uniqueComponents and seenCounts is a good approach to avoid O(n²) complexity when the same component block appears multiple times. However, the fallback case at lines 133-136 uses the old inefficient findIndex approach. If this fallback is truly unreachable as the comment suggests ("shouldn't happen"), consider throwing an error instead to make this explicit and help catch bugs during development.

Suggested change

// Fallback: shouldn't happen, but handle it

const index = order.components.findIndex((c) => isEqual(c, curr));

computedComponents[i] = _componentBlockToSequence(curr, latinSquareObject, `${path}-${index}`, config) as unknown as ComponentBlock;

// This branch should be unreachable: every non-string, non-array component

// in computedComponents is expected to have a corresponding entry in uniqueComponents.

// Throw an error to surface potential logic bugs during development.

throw new Error('Unexpected component encountered while mapping computedComponents; no matching entry in uniqueComponents.');

Copilot · 2026-01-09T16:26:19Z

src/utils/handleRandomSequences.tsx

+    .map((p) => {
+      const usageCount = pathUsageCounts[p] || 1;
+      // Generate multiple latin squares if needed and concatenate them
+      const rows: string[][] = [];
+      for (let i = 0; i < usageCount; i += 1) {
+        rows.push(...generateLatinSquare(config, p));
+      }
+      return { [p]: rows };
+    })


The pre-generation of multiple Latin square rows based on usage count is a good optimization to avoid refilling mid-sequence. However, the loop at lines 322-324 could potentially generate a very large number of rows if usageCount is high, which could impact memory usage. Consider adding a reasonable upper limit or warning if usageCount exceeds an expected threshold.

Copilot · 2026-01-09T16:26:19Z

src/components/interface/StepsPanel.tsx

+    for (let i = 0; i < newFlatTree.length; i += 1) {
+      const item = newFlatTree[i];
+      if (item.order !== undefined) { // It's a block
+        const startIndentLevel = item.indentLevel;
+        let endIndex = i + 1;
+        while (endIndex < newFlatTree.length && newFlatTree[endIndex].indentLevel > startIndentLevel) {
+          endIndex += 1;
+        }
+        item.childrenRange = { start: i + 1, end: endIndex };
+      }
+    }


The childrenRange pre-computation (lines 417-427) calculates the range of children for each block to enable O(1) collapse/expand operations. However, this assumes the fullFlatTree never changes during the component's lifecycle. If fullFlatTree is regenerated (which happens in the useEffect whenever dependencies change), these pre-computed ranges could become stale. Verify that renderedFlatTree is always based on the current fullFlatTree and ranges are recalculated appropriately.

Copilot · 2026-01-09T16:26:19Z

src/components/interface/StepsPanel.tsx

+      for (let i = start; i < end; i += 1) {
+        const item = fullFlatTree[i];
+
+        // Only add items that are direct children
+        if (item.indentLevel === startIndentLevel + 1) {
+          itemsToInsert.push(item);
+        } else if (item.indentLevel > startIndentLevel + 1) {
+          // This is a nested child - check if its parent block is in itemsToInsert
+          // Find the most recent block at the parent level
+          let shouldInclude = false;
+          for (let j = itemsToInsert.length - 1; j >= 0; j -= 1) {
+            const potentialParent = itemsToInsert[j];
+            if (potentialParent.indentLevel < item.indentLevel
+                && potentialParent.indentLevel === item.indentLevel - 1
+                && potentialParent.order !== undefined) {
+              // This item's parent is in the list, so include it
+              shouldInclude = true;
+              break;
+            }
+            if (potentialParent.indentLevel < item.indentLevel - 1) {
+              break;
+            }
+          }
+          if (shouldInclude) {
+            itemsToInsert.push(item);
+          }
+        }


The expandBlock function has complex nested logic for determining which items to insert (lines 475-501). The logic checks if nested children should be included based on whether their parent block is in itemsToInsert. However, this logic assumes all blocks at the same level will be expanded, which may not be correct if there are multiple collapsed blocks at different levels. This could lead to incorrect rendering when expanding blocks with complex nested structures.

Copilot · 2026-01-09T16:26:22Z

src/store/types.ts

+  order: ComponentBlock['order'] | 'dynamic';
  components: (string | Sequence)[];
-  skip?: SkipConditions;
+  skip: SkipConditions;


The skip field has been changed from optional (skip?: SkipConditions) to required (skip: SkipConditions). This is a breaking change that could cause runtime errors if any code creates Sequence objects without specifying the skip field. Verify that all places that create Sequence objects have been updated to provide this field, or consider making it optional with a default value to maintain backward compatibility.

Suggested change

skip: SkipConditions;

skip?: SkipConditions;

Copilot · 2026-01-09T16:26:22Z

src/utils/getSequenceFlatMap.ts

+    return {
+      ...order, orderPath, components: [], skip: [],
+    };
  }
  return {
-    ...order, orderPath, order: order.order, components: order.components.map((o, i) => addPathToComponentBlock(o, `${orderPath}-${i}`)),
+    ...order, orderPath, order: order.order, components: order.components.map((o, i) => addPathToComponentBlock(o, `${orderPath}-${i}`)), skip: order.skip || [],
  };


The addPathToComponentBlock function now always includes skip: [] in the returned object for dynamic blocks and uses skip: order.skip || [] for other blocks. While this ensures the skip field is always present (addressing the type change), it changes the behavior for blocks where skip was previously undefined. Code that checks for the presence of skip conditions using truthiness (e.g., if (block.skip)) will now always evaluate to true. Ensure this doesn't break any conditional logic elsewhere in the codebase.

Copilot · 2026-01-09T16:26:23Z

src/utils/handleRandomSequences.tsx

+function _countPathUsage(
+  order: StudyConfig['sequence'],
+  pathCounts: Record<string, number>,
+  path: string,
+): void {
+  if (isDynamicBlock(order)) {
+    return;
+  }

-  let locationInSequence: Partial<ComponentBlock> | Partial<DynamicBlock> | string = {};
-  pathArr.forEach((p) => {
-    if (p === 'root') {
-      locationInSequence = config.sequence;
-    } else {
-      if (isDynamicBlock(locationInSequence as StudyConfig['sequence'])) {
-        return;
+  if (order.order === 'latinSquare') {
+    pathCounts[path] = (pathCounts[path] || 0) + 1;
+  }
+
+  // Get the components that will actually be processed
+  let computedComponents = order.components;
+
+  // Apply numSamples if present
+  if (order.numSamples !== undefined) {
+    computedComponents = computedComponents.slice(0, order.numSamples);
+  }
+
+  // Count recursively for nested blocks
+  // Pre-build a list of unique components with their indices (same approach as _componentBlockToSequence)
+  const uniqueComponents: Array<{ component: ComponentBlock | DynamicBlock; indices: number[] }> = [];
+
+  for (let j = 0; j < order.components.length; j += 1) {
+    const comp = order.components[j];
+    if (typeof comp !== 'string' && !Array.isArray(comp) && !isDynamicBlock(comp)) {
+      // Find if we've already seen this component (by value)
+      let found = false;
+      for (const unique of uniqueComponents) {
+        if (isEqual(unique.component, comp)) {
+          unique.indices.push(j);
+          found = true;
+          break;
+        }
+      }
+      if (!found) {
+        uniqueComponents.push({ component: comp, indices: [j] });
      }
-      locationInSequence = (locationInSequence as ComponentBlock).components[+p];
    }
-  });
+  }

-  const options = (locationInSequence as ComponentBlock).components.map((c: unknown, i: number) => (typeof c === 'string' ? c : `_componentBlock${i}`));
-  shuffle(options);
-  const newSquare: string[][] = latinSquare<string>(options, true);
-  return newSquare;
+  // Track how many times we've seen each unique component
+  const seenCounts = new Map<ComponentBlock | DynamicBlock, number>();
+
+  for (let i = 0; i < computedComponents.length; i += 1) {
+    const curr = computedComponents[i];
+    if (typeof curr !== 'string' && !Array.isArray(curr) && !isDynamicBlock(curr)) {
+      // Find the matching unique component
+      let matchedUnique = null;
+      for (const unique of uniqueComponents) {
+        if (isEqual(unique.component, curr)) {
+          matchedUnique = unique;
+          break;
+        }
+      }
+
+      if (matchedUnique) {
+        const seenCount = seenCounts.get(matchedUnique.component) || 0;
+        const actualIndex = matchedUnique.indices[seenCount] ?? matchedUnique.indices[0];
+        seenCounts.set(matchedUnique.component, seenCount + 1);
+
+        _countPathUsage(curr, pathCounts, `${path}-${actualIndex}`);
+      } else {
+        // Fallback: shouldn't happen, but handle it
+        _countPathUsage(curr, pathCounts, `${path}-0`);
+      }
+    }
+  }
+}


The _countPathUsage function mirrors the logic from _componentBlockToSequence to ensure accurate counting. While this is necessary, it creates code duplication that could become a maintenance burden if the sequence generation logic changes. Consider extracting the shared iteration logic into a helper function that both functions can use, or add comprehensive tests to ensure these two functions stay in sync.

Copilot · 2026-01-09T16:26:23Z

src/components/interface/StepsPanel.tsx

+          const correctIncorrectIcon = correctAnswer && componentAnswer && componentAnswer?.endTime > -1
+            ? (correct
+              ? <IconCheck size={16} style={{ marginRight: 4, flexShrink: 0 }} color="green" />
+              : <IconX size={16} style={{ marginRight: 4, flexShrink: 0 }} color="red" />
+            )
+            : null;


The check for correctIncorrectIcon uses componentAnswer?.endTime > -1 to determine if the component was completed. However, this condition might not accurately represent whether the participant actually answered the component, especially if endTime could be 0 or other edge values. Consider using a more explicit check like componentAnswer?.endTime !== undefined && componentAnswer?.endTime >= 0.

Copilot · 2026-01-09T16:26:23Z

src/components/interface/AppAside.tsx

-        component={ScrollArea}
        p="xs"
        pt={4}
+        style={{ display: 'flex', flexDirection: 'column', overflow: 'hidden' }}


The ScrollArea component has been removed and replaced with flexbox layout using display: 'flex', flexDirection: 'column', overflow: 'hidden'. While this change is necessary for the virtualization to work properly, removing the ScrollArea might affect the scrolling behavior and styling. Ensure that the virtualized list inside StepsPanel properly handles scrolling and that the overall layout behaves correctly on different screen sizes.

Suggested change

style={{ display: 'flex', flexDirection: 'column', overflow: 'hidden' }}

style={{ display: 'flex', flexDirection: 'column', overflow: 'hidden', overflowY: 'auto' }}

JackWilb added 2 commits December 10, 2025 21:09

Update the StepsPanel code to render iteratively instead of recursively

f76534c

Add virtualized rendering to the steps panel using @tanstack/virtual

48e2bff

JackWilb linked an issue Dec 11, 2025 that may be closed by this pull request

Study browser is slow for lots of trials #778

Open

JackWilb added 9 commits January 7, 2026 19:20

Optimize the new steps panel

e6c8428

Add in the blocks that were excluded through randomization

31a9bff

Handle excluded components, don't count interruptions

e96ccd7

Handle generating larger latin squares

42e51c2

Speed up latin square generation

6ec4313

Fix bug with example full VLAT

a4fecdc

Fix performance and virtualization issues in the steps panel

556cfb0

Fix navigation in the analysis platform

d004384

Merge branch 'dev' into virtualized-steps-panel

3ee20a0

JackWilb marked this pull request as ready for review January 8, 2026 04:33

JackWilb added 2 commits January 7, 2026 21:34

Set overscan to an optimal value

07c657a

Fix test

03886d0

JackWilb requested review from Copilot and jaykim1213 January 9, 2026 16:19

Copilot started reviewing on behalf of JackWilb January 9, 2026 16:20 View session

Copilot AI reviewed Jan 9, 2026

View reviewed changes

-        // Fallback: shouldn't happen, but handle it
-        const index = order.components.findIndex((c) => isEqual(c, curr));
-        computedComponents[i] = _componentBlockToSequence(curr, latinSquareObject, `${path}-${index}`, config) as unknown as ComponentBlock;
+        // This branch should be unreachable: every non-string, non-array component
+        // in computedComponents is expected to have a corresponding entry in uniqueComponents.
+        // Throw an error to surface potential logic bugs during development.
+        throw new Error('Unexpected component encountered while mapping computedComponents; no matching entry in uniqueComponents.');

	style={{ display: 'flex', flexDirection: 'column', overflow: 'hidden' }}
	style={{ display: 'flex', flexDirection: 'column', overflow: 'hidden', overflowY: 'auto' }}

Virtualize the steps panel (study browser) #984

Are you sure you want to change the base?

Virtualize the steps panel (study browser) #984

Uh oh!

Conversation

JackWilb commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Does this PR close any open issues?

Give a longer description of what this PR addresses and why it's needed

Provide pictures/videos of the behavior before and after these changes (optional)

Are there any additional TODOs before this PR is ready to go?

Uh oh!

github-actions bot commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JackWilb commented Jan 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JackWilb commented Dec 11, 2025 •

edited

Loading

github-actions bot commented Jan 8, 2026 •

edited

Loading