fix: bound client upload concurrency and surface per-file outcomes (issue #8 follow-up to #294)

🟢 open · #381 · open-legal-products/mike ← amal66/mike · opened 15d ago by amal66 · +214-22 across 5 files · ↗ on GitHub

From the PR description

fix: bound upload concurrency and report per-file outcomes at every multi-file upload site

Follow-up to issue #8 (bulk uploads → Cloudflare 524s + phantom CORS errors) on the client side, complementing #294's server-side fix (async conversion). One commit, frontend-only.

The problem on main

Three multi-file upload sites (project chat explorer drops, NewTRModal, AddDocumentsModal) share two defects:

  1. Unbounded fan-out. A 20-file drop opens 20 simultaneous multipart requests via Promise.all - each carrying a full document that main's backend converts inline. Against production this is exactly the load pattern behind issue #8's 524s; even locally it races the rate limiter.
  2. All-or-nothing results. Promise.all rejects on the first failure and discards every other file's result - files that the server accepted (201) are never attached or shown, becoming phantom documents, and the user gets no per-file account of what failed.

A fourth gap: drag-and-drop skipped the file-type validation the picker had.

Base-case replication (main, 2026-08-24, live-recorded)

  1. Stack up (gateway :54721, backend :3001, frontend dev on :3000), log in, open Assistant → "+" → Add Documents → Upload.
  2. Select 14 files at once: 12 small PDFs + 1 PDF over the 100 MB multer limit (MAX_UPLOAD_SIZE_BYTES, deterministic 413) + 1 unsupported .xyz.
  3. Observe (recorded below): the Network tab fires 13 uploads simultaneously (measured max-in-flight: 13). Twelve return 201, the oversized one 413 - and the modal shows only the generic unsupported-type banner. None of the 12 uploaded PDFs appear in the list; the Promise.all rejection threw their results away (console.error("Upload failed:") is the only trace). Twelve phantom documents now exist server-side.

main: unbounded parallel uploads, silent batch failure

Settled state on main - 13-wide burst, no failure warning, 12 successful uploads discarded:

main settled

PR replication (this branch, same stack/backend/files, live-recorded)

Same 14-file selection:

  1. Uploads run through settleWithConcurrency(files, DOCUMENT_UPLOAD_CONCURRENCY /* = 2 */) - measured max-in-flight ≤ 3 (the bound is 2; the third is measurement skew at the handoff instant), same 12×201 + 1×413 statuses.
  2. All 12 successful PDFs appear and are auto-selected ("12 selected", Confirm enabled).
  3. The warning banner names both failures: the unsupported .xyz and "Could not upload oversized-appendix.pdf. Please try again." - per-file outcomes via the new formatFailedUploadWarning / combineUploadWarnings.
  4. Drag-and-drop now runs the same partitionSupportedDocumentFiles validation as the picker at all three sites.

PR: bounded uploads, per-file outcomes

Settled state on the branch - every success kept, every failure named:

pr settled

Both recordings were driven by the same scripted session (login → modal → one 14-file selection) against the same backend; only the frontend build differs. Request concurrency was measured in-browser via Playwright request/response hooks (result.json alongside each recording: maxInFlight: 13 on main vs 3 on the branch).

What the change is

  • Ports #380's settleWithConcurrency (already proven at the DocTable collection-upload path) to the three remaining unbounded sites: project chat explorer ([chatId]/page.tsx), NewTRModal, AddDocumentsModal.
  • Promise.all → settled results: successes attach, failures are collected per file.
  • New formatFailedUploadWarning + combineUploadWarnings in documentUploadValidation.ts (unit-tested) merge unsupported-type and failed-upload messages into one honest banner.
  • Drag-drop paths get the picker's type validation.

Tradeoffs & design decisions

  • No auto-retry. A failed file is reported, not silently retried - retry policy belongs with the server-side queue work (#294), and client retries against an overloaded backend amplify the original incident.
  • No batch cap. Any number of files may be selected; only in-flight parallelism is bounded. A hard cap would be a product decision, and the 2-wide bound already removes the thundering herd.
  • Concurrency constant shared, not per-site. All sites reuse DOCUMENT_UPLOAD_CONCURRENCY = 2 from #380 rather than inventing per-surface tuning - one number to reason about until measurement says otherwise.
  • The oversized file still travels to the server to be refused (413). Client-side size pre-checks were left out deliberately: the server limit is the source of truth, and duplicating it risks drift.

Testing

  • frontend: tsc clean; vitest suite green including new documentUploadValidation.test.ts cases (warning formatting, combination, partition).
  • Live A/B above (2026-08-24): deterministic 413 via a 101 MB file; identical backend (:3001) and database for both runs.
  • All CI checks green on the branch.

🤖 Generated with Claude Code

https://claude.ai/code/session_0132PfwQ6VviSeCRgdGhiq9Z

Our analysis

Bound concurrent document uploads — read the full analysis →

Think the analysis missed something the PR description covers?

Capture this PR into my fork

Download a Markdown prompt that tells Claude how to port every commit in this PR into your working tree. Run it via claude -p < capture-pull-381.md from inside the repo you want the changes in.

⬇ Download capture-pull-381.md