fix(tables): fix bulk ops truncation for tables larger than one page#4532
fix(tables): fix bulk ops truncation for tables larger than one page#4532waleedlatif1 wants to merge 9 commits intostagingfrom
Conversation
Bulk operations (column-header delete, select-all copy/cut/delete/run) were silently truncated to the first 1000 rows because handlers only iterated the loaded pages from useInfiniteQuery. Fix: - Extract tableRowsInfiniteOptions factory (infiniteQueryOptions) so the hook and imperative drain share the same typed cache key - Add background drain via useEffect watching hasNextPage/isFetchingNextPage — chains fetchNextPage until getNextPageParam returns undefined - Add ensureAllRowsLoaded to use-table: reads cache via getQueryData + calls fetchNextPage in a while loop until the last page is partial - Await ensureAllRowsLoaded at every kind:'all' bulk-op entry point in table-grid (column delete, copy, cut, action-bar delete/run) - Add chunkBatchUpdates to send updates in MAX_BULK_OPERATION_SIZE=1000 chunks so server validation never rejects oversized batches - Fix undo-redo: make executeAction async and chunk clear-cells, update-cells, and delete-column cell-restore with mutateAsync loops Tests: 41 passing across use-table, tables queries, and use-table-undo
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Adds shared infinite-query option/key helpers ( Hardens bulk updates/undo to respect server batch limits by chunking Reviewed by Cursor Bugbot for commit d4719f3. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes bulk operations (clear, copy, cut, delete, run) being silently truncated to 1000 rows on multi-page tables by introducing a background page drain and an
Confidence Score: 3/5The bulk-op drain and chunking logic is correct, but the incidental type="number" change to the inline cell editor introduces a regression blocking negative and decimal number entry. The inline editor change is a regression on the critical data-entry path: with a React-controlled type="number" input, browsers return empty string from e.target.value for incomplete states like a lone minus sign or trailing decimal, immediately clearing the character the user just typed. Entering -5 produces 5, and 1.5 typed digit-by-digit produces 15. This affects every table with a number column. apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.tsx — the type="number" controlled input change needs to be reverted or replaced with an uncontrolled approach for intermediate states. Important Files Changed
Reviews (4): Last reviewed commit: "fix(tables): push undo before chunkBatch..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@greptile |
|
@cursor review |
…ipboard NotAllowedError
|
@cursor review |
|
@greptile |
|
@cursor review |
|
@greptile |
| <input | ||
| ref={inputRef} | ||
| type='text' | ||
| value={draft} | ||
| type={isNumber ? 'number' : 'text'} | ||
| step={isNumber ? 'any' : undefined} | ||
| value={draft ?? ''} | ||
| onChange={(e) => setDraft(e.target.value)} |
There was a problem hiding this comment.
type="number" controlled input silently discards intermediate states
Browsers return '' from e.target.value whenever the current text is not a valid, complete number — this includes the intermediate states - (negative sign only), 1. (trailing decimal), and 1e (partial scientific notation). Because draft is set to '' on every such event, React immediately overwrites the DOM value back to '', so the character the user just typed visibly disappears.
Concrete failure: open a number-column cell for inline editing, type -5. After pressing -, e.target.value is '', draft becomes '', and the input clears; the 5 then lands on the now-empty field as 5, not -5. Similarly, 1.5 typed digit-by-digit becomes 15 because the decimal point is discarded.
The standard workaround is to keep type="text" for the draft state and rely on cleanCellValue (already used in doSave) for validation — the call-site already converts the string to a number or null before persisting, so no server-side change is needed. Alternatively, use inputMode="decimal" with type="text" to get mobile numeric keyboards without the intermediate-state problem.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d4719f3. Configure here.
| cancelled = true | ||
| if (timeoutId !== null) clearTimeout(timeoutId) | ||
| } | ||
| }, [enabled, workspaceId, tableId, pageSize, filter, sort, queryClient, queryKey]) |
There was a problem hiding this comment.
Polling loop runs indefinitely even without running executions
Medium Severity
The polling useEffect in useInfiniteTableRows unconditionally reschedules a setTimeout every 2 seconds regardless of whether any pages have running executions. Every open table view — including tables with no workflows at all — continuously polls the cache and reschedules indefinitely. The tick function only skips the network fetch when dirty.length === 0, but the loop itself never pauses or stops. This wastes CPU cycles doing cache reads and timer bookkeeping across all mounted table views.
Reviewed by Cursor Bugbot for commit d4719f3. Configure here.
| onError: (error) => { | ||
| if (isValidationError(error)) return | ||
| logger.error('Failed to upload CSV:', error) | ||
| toast.error(error.message, { duration: 5000 }) |
There was a problem hiding this comment.
CSV upload validation errors silently swallowed without user feedback
Medium Severity
The mutation's onError now returns early for isValidationError(error) without showing a toast, and the calling code in tables.tsx removed the failed array and its associated error toast. When a CSV import fails with a validation error (e.g., exceeding row limits or schema violations), the user sees no feedback — the error is only logged server-side.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit d4719f3. Configure here.
| void chunkBatchUpdates(batchUpdates, batchUpdateAsyncRef.current).catch((error) => { | ||
| logger.error('Failed to cut selected cells', { error }) | ||
| toast.error('Failed to cut — please try again') | ||
| }) |
There was a problem hiding this comment.
Range-selection cut path doesn't drain pages for column selection
Medium Severity
The range-selection Cut path (when rowSelectionIsEmpty is true) operates on currentRows which only contains loaded pages. When a column is selected via column-header click, isColumnSelection is true but row selection is empty, so the code falls into this range-selection branch. Unlike the column-selection Delete handler (which calls ensureAllRowsLoaded), this Cut path silently truncates to loaded rows — only cutting visible data and leaving unloaded rows intact on tables larger than one page.
Reviewed by Cursor Bugbot for commit d4719f3. Configure here.
1 participant
Summary
Type of Change
Testing
Tested manually — verified all rows clear/copy/cut/delete correctly on tables >1000 rows. All unit tests passing.
Checklist