Skip to content

fix(parsers): yield the event loop during PDF extraction and CSV row coercion - #9311

Merged
waleedlatif1 merged 2 commits into
stagingfrom
fix/parse-yield-pdf-csv
Oct 22, 2026
Merged

waleedlatif1 merged 2 commits into
stagingfrom
fix/parse-yield-pdf-csv

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • PDF extraction: pdf.js runs its fake worker on the request thread and resolves every message as a microtask, so neither the page loop nor a page's streamTextContent read loop ever let timers or I/O run. One shared time-sliced check (createEventLoopSlicer, setImmediate from node:timers/promises) now yields a macrotask once 10ms have passed since the last yield, in the page loop, inside the stream read loop (covers one page with a huge content stream), and in page assembly, between its whole-document passes and per page (replaces the fixed sleep(0) every 32 pages).
  • Table CSV append/replace: rows are coerced in slices of 2,000 with a yield between slices (coerceRowsForTable is a 1:1 map, so rows and cellsRejected are identical).
  • Row limit is now checked right after parsing for both append and replace, before coercion and before claiming the job slot.

User-visible changes:

  • An over-limit replace now fails earlier with a validation message ("Replace would exceed table row limit (N). The file has X rows, Y over.", still HTTP 400) instead of the in-transaction row-limit error, and without claiming the job slot.
  • An over-limit file now reports the row-limit validation error before a concurrent-import conflict (previously the slot claim ran first).
  • PDF extraction output and CSV coercion output are unchanged (hashes below).

Type of Change

  • Bug fix

Testing

  • New regression tests, each proven red on pre-fix code:
    • pdf-parser-cancellation.test.ts: a timer runs between CPU-bound pages; a timer runs while one page streams many chunks (red with only the inner read-loop yield removed)
    • import.test.ts: a timer runs during coercion of a 50,000-row file before the write; an over-limit append/replace returns the row-limit validation error while the slot is held by another job (a within-limit replace still gets conflict)
  • Ran pdf-parser-cancellation, pdf-parser, pdf-parser-structure, import tests (32 passed), bun run lint. Full type-check/audits/test suite left to CI.
  • Local before/after (generated fixtures; max gap between 1ms interval ticks while the call runs, 3 runs):
Fixture Before max block After max block Wall time Output hash
400-page text PDF 667–701ms 47–51ms ~720ms both identical (normal + complete mode)
1 page, 30,000 text ops 258–273ms 35–42ms ~270ms both identical (normal + complete mode)
23 MB CSV, 5 typed columns, I/O-paced 64KB chunks 265ms 74ms ~1.05s both coerced rows + cellsRejected identical

The remaining PDF block is a single whole-document assembly pass (furniture suppression or word collection); the CSV residual includes the bench's own hashing of the rows.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing (new tests pass the test-audit authoring gate)
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…coercion

pdf.js runs its fake worker on the request thread and resolves every message
as a microtask, so the page loop and the text-stream read loop never let
timers or I/O run. A shared time-sliced check now yields a macrotask once 10ms
have passed, across page reads, stream reads, and page assembly.

Table CSV append/replace coerces rows in slices of 2,000 with a yield between
slices, and checks the plan row limit right after parsing for both modes,
before coercion and before claiming the table job slot.
@vercel

vercel Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Oct 22, 2026 8:31pm UTC

Request Review

@greptile-apps

greptile-apps Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The changes appear safe to merge; no blocking issues were found.

Summary

This PR lets other requests and timers run during PDF extraction and large CSV imports.

  • PDF extraction gives timers and I/O a turn while it reads and assembles pages.
  • Existing-table CSV imports coerce rows in slices and yield between them.
  • Append and replace imports check the row limit before claiming the job slot.

No actionable new issues were found. No previous review threads were supplied.

Reviews (2) · Last reviewed commit: "fix(parsers): yield between PDF assembly..." · Reviewed by Greptile

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 4 files

Heads up: you’re close to your flex budget. Increase your flex budget so reviews don’t pause.

Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/lib/file-parsers/pdf-parser.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 4 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Heads up: you’re close to your flex budget. Increase your flex budget so reviews don’t pause.

Turn on auto-fix | Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit 22864e9 into staging Oct 22, 2026
65 of 66 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/parse-yield-pdf-csv branch October 22, 2026 21:10

This branch was previously deployed

1 inactive deployment
Preview — f16120fb Deployed Oct 22, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant