Skip to content

feat: support concurrent chunk uploads#154

Merged
TorstenDittmann merged 4 commits into
mainfrom
concurrent-chunk-uploads-1-9-x-minimal
May 21, 2026
Merged

feat: support concurrent chunk uploads#154
TorstenDittmann merged 4 commits into
mainfrom
concurrent-chunk-uploads-1-9-x-minimal

Conversation

@TorstenDittmann
Copy link
Copy Markdown
Contributor

This PR updates the SDK to support concurrent chunk uploads.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 21, 2026

Greptile Summary

This PR replaces the sequential chunked-upload loop with a bounded concurrent worker pool (up to 8 in-flight requests) for both the Node.js InputFile path and the browser File path. The first chunk is sent alone to obtain the server-assigned upload ID; subsequent chunks are dispatched through a shared queue that up to 8 async workers drain in parallel.

  • The worker pool pattern is correctly coordinated through JavaScript's single-threaded event loop — queue.shift() is always called synchronously before the next await, so no TOCTOU issue exists on the shared queue.
  • Both upload paths contain a dead-code if (totalChunks === 1) guard that can never be reached because the earlier size <= CHUNK_SIZE early-return already handles that case.
  • The ping() return type was changed from Promise<string> to Promise<unknown> — an unrelated breaking TypeScript API change.

Confidence Score: 3/5

The concurrent upload rewrite has known unresolved correctness issues in both upload paths that warrant a fix before merging.

Two distinct issues flagged in earlier review rounds remain unaddressed in this diff: workers are not cancelled when the first failure occurs, and the fallback return value can silently be a stale first-chunk response. Both affect the core upload path and appear in both the InputFile and browser File code paths.

src/client.ts — both the InputFile and browser File worker-pool sections need attention for the error-propagation and return-value fallback concerns raised in the prior review threads.

Important Files Changed

Filename Overview
src/client.ts Rewrites the chunked-upload logic for both InputFile and browser File paths to use a concurrent worker pool (up to 8 parallel requests). Unresolved concerns from prior review threads (zombie workers after the first failure, and stale lastResponse fallback) are still present; new in this review: unreachable totalChunks === 1 guards in both paths and an unrelated breaking narrowing of ping() return type.

Reviews (4): Last reviewed commit: "feat: support concurrent chunk uploads" | Re-trigger Greptile

Comment thread src/client.ts
Comment thread src/client.ts
Comment thread src/client.ts Outdated
Comment thread src/client.ts Outdated
Comment thread src/client.ts
Comment thread src/client.ts
@TorstenDittmann TorstenDittmann merged commit 64c7e55 into main May 21, 2026
2 checks passed
@TorstenDittmann TorstenDittmann deleted the concurrent-chunk-uploads-1-9-x-minimal branch May 21, 2026 18:21
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.

2 participants