fix(web): make bulk Loom CSV import resilient to batch failures - #2079
fix(web): make bulk Loom CSV import resilient to batch failures#2079richiemcilroy wants to merge 1 commit into
Conversation
A failed importFromLoomCsv batch call previously aborted the whole run: every remaining batch was never attempted and never appeared in the results table, so large CSVs could silently drop most rows. - Isolate failures per batch: a failed batch is recorded as failed rows (with the error message) and the loop continues. - Account for every row: rows never attempted are appended to the results as "Not attempted." instead of vanishing, including when the run stops early on a blocked response or unexpected error. - Back off once per batch when a failure looks like rate limiting (30s wait, single retry of only the rate-limited rows) before recording it and moving on. Rate-limited rows are rejected server side before any import starts, so retrying them cannot duplicate. - Add a Download Results button that exports the full per-row results as a CSV (re-uploadable headers) so failed rows can be filtered and retried safely.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
| function escapeCsvValue(value: string) { | ||
| if (/[",\n\r]/.test(value)) { | ||
| return `"${value.replace(/"/g, '""')}"`; | ||
| } | ||
| return value; | ||
| } |
There was a problem hiding this comment.
Neutralize formulas in CSV cells
Formula-prefixed email and space-name values pass the current validation and are written directly to the downloadable CSV. Opening that file in formula-evaluating spreadsheet software interprets those values as formulas rather than literal data, so the exporter should neutralize spreadsheet control prefixes while preserving the intended retry workflow.
How this was verified: The imported fields flow through buildResultsCsv to escapeCsvValue, which escapes CSV delimiters but does not neutralize leading formula characters.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/app/(org)/dashboard/import/loom/ImportLoomPage.tsx
Line: 173-178
Comment:
**Neutralize formulas in CSV cells**
Formula-prefixed email and space-name values pass the current validation and are written directly to the downloadable CSV. Opening that file in formula-evaluating spreadsheet software interprets those values as formulas rather than literal data, so the exporter should neutralize spreadsheet control prefixes while preserving the intended retry workflow.
**How this was verified:** The imported fields flow through `buildResultsCsv` to `escapeCsvValue`, which escapes CSV delimiters but does not neutralize leading formula characters.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Problem
The bulk Loom CSV importer sends rows to
importFromLoomCsvin batches of 10, but the entire multi-batch loop ran inside a single try/catch. One failed batch call (server error, timeout, network blip, rate-limit rejection) aborted the loop: every remaining batch was never attempted and never appeared in the results table, not even as failed. On large CSVs this silently dropped most rows - the UI reported only the imports from the batches that ran, with no record of the rest anywhere.Bulk imports also share a per-user rate limit, which large CSVs can exhaust mid-run, making this failure mode common rather than exotic.
Fix
All changes are client-side in
ImportLoomPage.tsx; the server action, rate-limit code, and single-video import path are untouched.Regression analysis
What could have regressed, and why it can't:
router.refresh(). The not-attempted fill is a no-op when every row has a result (early return before any sort), and the retry block never executes when no row failed with a rate-limit error. The only visible addition on success is the Download Results button.importedVideosdedupe rejects any Loom ID already imported for the org. Successful rows in a partially rate-limited batch are never resent.actions/loom.tsis untouched; server-action tests (loom-import.test.ts, 12 tests) pass unchanged.ImportLoomPage.tsx, and the component's only consumer is its route page. The single-video import handler, CSV parsing/mapping/preview, and shared@cap/uicomponents are behaviorally untouched (one layout class,flex-wrap, added to the results-card header so the new button wraps on narrow screens).Verification
pnpm typecheck(next typegen +tsc -b): passpnpm exec biome checkon the changed file: cleanvitest run __tests__/unit/loom-import.test.ts: 12/12 pass (server action unchanged)Greptile Summary
The PR makes bulk Loom CSV imports continue after individual batch failures, retries rate-limited rows once, accounts for unattempted rows, and adds a downloadable result report.
Confidence Score: 4/5
The PR appears safe to merge, with a non-blocking CSV-formula hardening issue in the new results download.
Batch and retry result accounting preserves submitted row numbers and failed outcomes, while the remaining concern is that imported email and space-name values can be emitted as active spreadsheet formulas.
Files Needing Attention: apps/web/app/(org)/dashboard/import/loom/ImportLoomPage.tsx
Security Review
The downloadable results CSV does not neutralize formula-prefixed email or space-name values before spreadsheet consumption.
Important Files Changed
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(web): make bulk Loom CSV import resi..." | Re-trigger Greptile