Table virtualization #14743

lucasbordeau · 2025-09-26T14:05:04Z

WIP

greptile-apps

Greptile Overview

Summary

This PR implements table virtualization for the record table to improve performance when handling large datasets. The implementation introduces a virtualized row system that renders only visible rows plus a buffer, dynamically fetches data as users scroll, and manages state through a treadmill pattern that reuses DOM elements.

Key Changes

Virtualization Core: RecordTableRowVirtualized component that renders rows based on virtual/real index mapping
Data Loading: RecordTableVirtualizedDataLoaderEffect handles progressive data fetching with batched UI updates
Scroll Management: RecordTableVirtualizedRowTreadmillEffect manages the scroll-to-virtualization mapping with proper debouncing
State Management: Multiple new state atoms track virtualization indices, fetch status, and scroll positions
Utility Function: New getRange utility added to twenty-shared for array operations

Issues Found

Debug output present: The virtualized row component contains debug output that should be removed before production
Missing error handling: Async data loading operations lack proper error handling which could cause silent failures
Hardcoded values: Magic numbers used instead of established constants for consistency
API design: getRange function has confusing parameter semantics that could lead to off-by-one errors

Confidence Score: 3/5

This PR introduces significant performance improvements but contains debug output and missing error handling that need attention
Score reflects the architectural complexity and solid implementation approach, but several issues must be addressed: debug output needs removal, async operations need error handling, and hardcoded values should use constants. The virtualization logic itself is well-structured with proper debouncing and state management.
Pay close attention to RecordTableRowVirtualized.tsx (debug output removal) and RecordTableVirtualizedDataLoaderEffect.tsx (error handling)

Important Files Changed

File Analysis

Filename	Score	Overview
packages/twenty-front/src/modules/object-record/record-table/virtualization/components/RecordTableRowVirtualized.tsx	2/5	Core virtualized row component with debug output that needs removal and hardcoded values that should use constants
packages/twenty-front/src/modules/object-record/record-table/virtualization/components/RecordTableVirtualizedDataLoaderEffect.tsx	3/5	Data loading logic with missing error handling for async operations that could cause silent failures
packages/twenty-shared/src/utils/array/getRange.ts	3/5	Utility function with confusing parameter naming that could lead to off-by-one errors

Sequence Diagram

sequenceDiagram
    participant User
    participant RecordTable
    participant VirtualBody
    participant DataLoader
    participant TreadmillEffect
    participant API
    
    User->>RecordTable: Initial load
    RecordTable->>VirtualBody: Mount virtualized body
    VirtualBody->>DataLoader: Initialize data loading
    DataLoader->>API: fetchRecordsLazy()
    API-->>DataLoader: records + totalCount
    DataLoader->>VirtualBody: resetVirtualizedRowTreadmill()
    DataLoader->>VirtualBody: setRecordTableData(records)
    
    User->>RecordTable: Scroll down
    RecordTable->>TreadmillEffect: handleScroll event
    TreadmillEffect->>TreadmillEffect: Calculate new visible range
    TreadmillEffect->>VirtualBody: Update realIndexByVirtualIndex mapping
    
    Note over TreadmillEffect: Checks if more data needed
    alt lastRealIndexSet > hasAlreadyFetchedUpToRealIndex
        TreadmillEffect->>DataLoader: Trigger fetch more
        DataLoader->>API: fetchMoreRecords(150)
        API-->>DataLoader: more records
        DataLoader->>DataLoader: Batch process records (10 at a time)
        loop For each batch
            DataLoader->>VirtualBody: Update recordIdByRealIndex state
            DataLoader->>DataLoader: Wait 50ms between batches
        end
    end
    
    VirtualBody->>RecordTable: Render virtualized rows
    RecordTable-->>User: Display updated table

_{46 files reviewed, 7 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2025-09-26T14:10:15Z

...c/modules/object-record/record-table/virtualization/components/RecordTableRowVirtualized.tsx

+        >
+          {virtualIndex}-{realIndex}-{pixelsFromTop}-{recordId}
+        </div>


logic: Debug output should not be in production code

Suggested change

>

{virtualIndex}-{realIndex}-{pixelsFromTop}-{recordId}

</div>

{/* Debug info removed for production */}

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/twenty-front/src/modules/object-record/record-table/virtualization/components/RecordTableRowVirtualized.tsx Line: 87:89 Comment: logic: Debug output should not be in production code ```suggestion {/* Debug info removed for production */} ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2025-09-26T14:10:16Z

...c/modules/object-record/record-table/virtualization/components/RecordTableRowVirtualized.tsx

+      <div
+        style={{
+          position: 'absolute',
+          left: 500,
+          top: 10,
+          zIndex: 4,
+          color: 'GrayText',
+          width: 500,
+        }}
+      >
+        {virtualIndex}-{realIndex}-{pixelsFromTop}-{recordId}
+      </div>


logic: Debug output should not be in production code

Suggested change

<div

style={{

position: 'absolute',

left: 500,

top: 10,

zIndex: 4,

color: 'GrayText',

width: 500,

}}

>

{virtualIndex}-{realIndex}-{pixelsFromTop}-{recordId}

</div>

{/* Debug info removed for production */}

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/twenty-front/src/modules/object-record/record-table/virtualization/components/RecordTableRowVirtualized.tsx Line: 119:130 Comment: logic: Debug output should not be in production code ```suggestion {/* Debug info removed for production */} ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2025-09-26T14:10:17Z

...c/modules/object-record/record-table/virtualization/components/RecordTableRowVirtualized.tsx

+        style={{
+          position: 'absolute',
+          top: pixelsFromTop,
+          height: 33,


style: Magic number 33 should use RECORD_TABLE_ROW_HEIGHT constant for consistency

Suggested change

height: 33,

height: RECORD_TABLE_ROW_HEIGHT,

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/twenty-front/src/modules/object-record/record-table/virtualization/components/RecordTableRowVirtualized.tsx Line: 76:76 Comment: style: Magic number `33` should use `RECORD_TABLE_ROW_HEIGHT` constant for consistency ```suggestion height: RECORD_TABLE_ROW_HEIGHT, ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2025-09-26T14:10:18Z

...c/modules/object-record/record-table/virtualization/components/RecordTableRowVirtualized.tsx

+      style={{
+        position: 'absolute',
+        top: pixelsFromTop,
+        height: 33,


style: Magic number 33 should use RECORD_TABLE_ROW_HEIGHT constant for consistency

Suggested change

height: 33,

height: RECORD_TABLE_ROW_HEIGHT,

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/twenty-front/src/modules/object-record/record-table/virtualization/components/RecordTableRowVirtualized.tsx Line: 116:116 Comment: style: Magic number `33` should use `RECORD_TABLE_ROW_HEIGHT` constant for consistency ```suggestion height: RECORD_TABLE_ROW_HEIGHT, ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2025-09-26T14:10:18Z

...ect-record/record-table/virtualization/components/RecordTableVirtualizedDataLoaderEffect.tsx

+        fetchMoreRecordsLazy(FETCH_MORE_NUMBER).then(async (result) => {
+          if (!isDefined(result?.error)) {
+            const records = result?.records;
+
+            const startingRealIndex =
+              hasAlreadyFetchedUpToRealIndexSnapshotValue ?? 0;
+
+            if (isDefined(records)) {
+              const pagingForUIUpdate = PAGING_FOR_UI_UPDATE;
+
+              const pages = Math.ceil(records.length / pagingForUIUpdate);
+
+              let indexOfCurrentRecordBatch = 0;
+
+              for (let page = 0; page < pages; page++) {
+                await new Promise<void>((res) =>
+                  setTimeout(() => res(), TIME_BETWEEN_UI_BATCH_UPDATE),
+                );
+
+                const startingRealIndexInThisPage =
+                  startingRealIndex + page * pagingForUIUpdate;
+
+                const endingRealIndexInThisPage =
+                  startingRealIndexInThisPage + pagingForUIUpdate;
+
+                for (
+                  let realIndexToSet = startingRealIndexInThisPage;
+                  realIndexToSet < endingRealIndexInThisPage;
+                  realIndexToSet++
+                ) {
+                  if (isDefined(records[indexOfCurrentRecordBatch])) {
+                    set(
+                      recordIdByRealIndexCallbackState({
+                        realIndex: realIndexToSet,
+                      }),
+                      records[indexOfCurrentRecordBatch].id,
+                    );
+                  }
+
+                  indexOfCurrentRecordBatch++;
+                }
+              }
+
+              set(
+                hasAlreadyFetchedUpToRealIndexCallbackState,
+                startingRealIndex + records.length,
+              );
+
+              set(
+                hasRecordTableFetchedAllRecordsCallbackState,
+                result?.data?.pageInfo.hasNextPage === false,
+              );
+            }
+          }
+
+          set(isFetchingMoreRecordsFamilyState(recordTableId), false);
+        });


logic: Missing error handling for async operations could cause silent failures

Suggested change

fetchMoreRecordsLazy(FETCH_MORE_NUMBER).then(async (result) => {

if (!isDefined(result?.error)) {

const records = result?.records;

const startingRealIndex =

hasAlreadyFetchedUpToRealIndexSnapshotValue ?? 0;

if (isDefined(records)) {

const pagingForUIUpdate = PAGING_FOR_UI_UPDATE;

const pages = Math.ceil(records.length / pagingForUIUpdate);

let indexOfCurrentRecordBatch = 0;

for (let page = 0; page < pages; page++) {

await new Promise<void>((res) =>

setTimeout(() => res(), TIME_BETWEEN_UI_BATCH_UPDATE),

);

const startingRealIndexInThisPage =

startingRealIndex + page * pagingForUIUpdate;

const endingRealIndexInThisPage =

startingRealIndexInThisPage + pagingForUIUpdate;

for (

let realIndexToSet = startingRealIndexInThisPage;

realIndexToSet < endingRealIndexInThisPage;

realIndexToSet++

) {

if (isDefined(records[indexOfCurrentRecordBatch])) {

set(

recordIdByRealIndexCallbackState({

realIndex: realIndexToSet,

}),

records[indexOfCurrentRecordBatch].id,

);

}

indexOfCurrentRecordBatch++;

}

}

set(

hasAlreadyFetchedUpToRealIndexCallbackState,

startingRealIndex + records.length,

);

set(

hasRecordTableFetchedAllRecordsCallbackState,

result?.data?.pageInfo.hasNextPage === false,

);

}

}

set(isFetchingMoreRecordsFamilyState(recordTableId), false);

});

fetchMoreRecordsLazy(FETCH_MORE_NUMBER).then(async (result) => {

try {

if (!isDefined(result?.error)) {

const records = result?.records;

const startingRealIndex =

hasAlreadyFetchedUpToRealIndexSnapshotValue ?? 0;

if (isDefined(records)) {

const pagingForUIUpdate = PAGING_FOR_UI_UPDATE;

const pages = Math.ceil(records.length / pagingForUIUpdate);

let indexOfCurrentRecordBatch = 0;

for (let page = 0; page < pages; page++) {

await new Promise<void>((res) =>

setTimeout(() => res(), TIME_BETWEEN_UI_BATCH_UPDATE),

);

const startingRealIndexInThisPage =

startingRealIndex + page * pagingForUIUpdate;

const endingRealIndexInThisPage =

startingRealIndexInThisPage + pagingForUIUpdate;

for (

let realIndexToSet = startingRealIndexInThisPage;

realIndexToSet < endingRealIndexInThisPage;

realIndexToSet++

) {

if (isDefined(records[indexOfCurrentRecordBatch])) {

set(

recordIdByRealIndexCallbackState({

realIndex: realIndexToSet,

}),

records[indexOfCurrentRecordBatch].id,

);

}

indexOfCurrentRecordBatch++;

}

}

set(

hasAlreadyFetchedUpToRealIndexCallbackState,

startingRealIndex + records.length,

);

set(

hasRecordTableFetchedAllRecordsCallbackState,

result?.data?.pageInfo.hasNextPage === false,

);

}

}

} catch (error) {

// Handle fetch errors gracefully

console.error('Error in virtualized data loading:', error);

}

set(isFetchingMoreRecordsFamilyState(recordTableId), false);

}).catch((error) => {

// Handle promise rejection

console.error('Failed to fetch more records:', error);

set(isFetchingMoreRecordsFamilyState(recordTableId), false);

});

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/twenty-front/src/modules/object-record/record-table/virtualization/components/RecordTableVirtualizedDataLoaderEffect.tsx Line: 152:208 Comment: logic: Missing error handling for async operations could cause silent failures ```suggestion fetchMoreRecordsLazy(FETCH_MORE_NUMBER).then(async (result) => { try { if (!isDefined(result?.error)) { const records = result?.records; const startingRealIndex = hasAlreadyFetchedUpToRealIndexSnapshotValue ?? 0; if (isDefined(records)) { const pagingForUIUpdate = PAGING_FOR_UI_UPDATE; const pages = Math.ceil(records.length / pagingForUIUpdate); let indexOfCurrentRecordBatch = 0; for (let page = 0; page < pages; page++) { await new Promise<void>((res) => setTimeout(() => res(), TIME_BETWEEN_UI_BATCH_UPDATE), ); const startingRealIndexInThisPage = startingRealIndex + page * pagingForUIUpdate; const endingRealIndexInThisPage = startingRealIndexInThisPage + pagingForUIUpdate; for ( let realIndexToSet = startingRealIndexInThisPage; realIndexToSet < endingRealIndexInThisPage; realIndexToSet++ ) { if (isDefined(records[indexOfCurrentRecordBatch])) { set( recordIdByRealIndexCallbackState({ realIndex: realIndexToSet, }), records[indexOfCurrentRecordBatch].id, ); } indexOfCurrentRecordBatch++; } } set( hasAlreadyFetchedUpToRealIndexCallbackState, startingRealIndex + records.length, ); set( hasRecordTableFetchedAllRecordsCallbackState, result?.data?.pageInfo.hasNextPage === false, ); } } } catch (error) { // Handle fetch errors gracefully console.error('Error in virtualized data loading:', error); } set(isFetchingMoreRecordsFamilyState(recordTableId), false); }).catch((error) => { // Handle promise rejection console.error('Failed to fetch more records:', error); set(isFetchingMoreRecordsFamilyState(recordTableId), false); }); ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2025-09-26T14:10:19Z

packages/twenty-shared/src/utils/array/getRange.ts

+export const getRange = (start: number, end: number) => {
+  return Array.from({ length: end }, (_, index) => index + start)
+}


logic: Function behavior is confusing - getRange(0, 300) generates 0-299, not 0-300 as might be expected

Suggested change

export const getRange = (start: number, end: number) => {

return Array.from({ length: end }, (_, index) => index + start)

}

export const getRange = (start: number, length: number) => {

return Array.from({ length }, (_, index) => index + start);

};

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/twenty-shared/src/utils/array/getRange.ts Line: 1:3 Comment: logic: Function behavior is confusing - `getRange(0, 300)` generates 0-299, not 0-300 as might be expected ```suggestion export const getRange = (start: number, length: number) => { return Array.from({ length }, (_, index) => index + start); }; ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2025-09-26T14:10:20Z

...ord/record-table/virtualization/components/RecordTableNoRecordGroupVirtualizedBodyEffect.tsx

+    totalNumberOfRecordsToVirtualize,
+  ]);
+
+  return <></>;


style: Use null instead of empty fragment when returning nothing

Suggested change

return <></>;

return null;

Context Used: Context - Avoid using fragments when there is only one child component in a return statement. (link)

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/twenty-front/src/modules/object-record/record-table/virtualization/components/RecordTableNoRecordGroupVirtualizedBodyEffect.tsx Line: 108:108 Comment: style: Use `null` instead of empty fragment when returning nothing ```suggestion return null; ``` **Context Used:** **Context -** Avoid using fragments when there is only one child component in a return statement. ([link](https://app.greptile.com/review/custom-context?memory=51414064-3127-4b1d-ad7e-62ce2c3739e9)) How can I resolve this? If you propose a fix, please make it concise.

seer-by-sentry · 2025-09-26T14:12:01Z

...ect-record/record-table/virtualization/components/RecordTableVirtualizedDataLoaderEffect.tsx

+                  if (isDefined(records[indexOfCurrentRecordBatch])) {
+                    set(
+                      recordIdByRealIndexCallbackState({
+                        realIndex: realIndexToSet,
+                      }),
+                      records[indexOfCurrentRecordBatch].id,
+                    );
+                  }
+
+                  indexOfCurrentRecordBatch++;
+                }


Potential bug: The loop for processing fetched records can iterate past the end of the records array if its length is not a multiple of PAGING_FOR_UI_UPDATE.

Description: When fetching more records for the virtualized table, the number of pages is calculated using Math.ceil(records.length / pagingForUIUpdate). The code then iterates a number of times equal to pages * pagingForUIUpdate. If the number of records returned from the fetch is not a multiple of pagingForUIUpdate, this calculation results in more iterations than available records. This causes an out-of-bounds access on the records array when records[indexOfCurrentRecordBatch] is called. While this doesn't crash the app, it results in undefined values being processed, leading to null record IDs being set in the state, which can cause empty rows or rendering issues in the table.

Suggested fix: The inner loop that processes records should not iterate a fixed number of times per page. Instead, it should only iterate up to the actual length of the records array. The loop's end condition should be capped by the number of available records to prevent accessing an index beyond the array's bounds.
_{severity: 0.55, confidence: 0.95}

_{Did we get this right? 👍 / 👎 to inform future reviews.}

github-actions · 2025-09-26T14:17:19Z

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:44916

This environment will automatically shut down when the PR is closed or after 5 hours.

lucasbordeau added 15 commits September 25, 2025 11:00

Ok

00397dc

WIP

e0e2d21

WIP

2e869e6

Ok

4ccb6d7

Ok

4436247

Fixed add new

5b26960

Fixed

d705656

Fixed record table

a2b10f8

Restored commented code

bf56081

Removed unused

bd173f4

Fixed body loading

301f153

Ok

3b328f3

Removed unused

aba3f21

Ok

bac6409

WIP

ff541c2

lucasbordeau marked this pull request as draft September 26, 2025 14:05

greptile-apps bot reviewed Sep 26, 2025

View reviewed changes

seer-by-sentry bot reviewed Sep 26, 2025

View reviewed changes

Finished loading in one effect

c98a7b4

lucasbordeau added the -PR: wip label Sep 26, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Table virtualization #14743

Table virtualization #14743

lucasbordeau commented Sep 26, 2025

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Sep 26, 2025

Uh oh!

greptile-apps bot Sep 26, 2025

Uh oh!

greptile-apps bot Sep 26, 2025

Uh oh!

greptile-apps bot Sep 26, 2025

Uh oh!

greptile-apps bot Sep 26, 2025

Uh oh!

greptile-apps bot Sep 26, 2025

Uh oh!

greptile-apps bot Sep 26, 2025

Uh oh!

seer-by-sentry bot Sep 26, 2025

Uh oh!

github-actions bot commented Sep 26, 2025 •

edited

Loading

Uh oh!

Uh oh!

Table virtualization #14743

Are you sure you want to change the base?

Table virtualization #14743

Conversation

lucasbordeau commented Sep 26, 2025

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Overview

Summary

Key Changes

Issues Found

Confidence Score: 3/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

seer-by-sentry bot Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Sep 26, 2025 •

edited

Loading