Skip to content

feat: support concurrent chunk uploads#95

Closed
TorstenDittmann wants to merge 1 commit into
mainfrom
concurrent-chunk-uploads-1-9-x
Closed

feat: support concurrent chunk uploads#95
TorstenDittmann wants to merge 1 commit into
mainfrom
concurrent-chunk-uploads-1-9-x

Conversation

@TorstenDittmann

Copy link
Copy Markdown
Contributor

This PR updates the SDK to support concurrent chunk uploads.

@greptile-apps

greptile-apps Bot commented May 21, 2026

Copy link
Copy Markdown

Greptile Summary

This PR upgrades the SDK's chunked file upload from a sequential loop to a concurrent pool (max 8 in-flight chunks), uploads the first chunk individually to obtain the server-assigned uploadId, then dispatches the remaining chunks in parallel. It also adds new services (Presences, Advisor, Usage), renames Project-scoped enums to their new Project-prefixed names, and adds several new model types.

  • Concurrent upload pool (src/client.ts): the first chunk is sent alone to obtain $id; remaining chunks are dispatched up to 8 at a time using a hand-rolled promise queue; progress and the final response are tracked via shared counters.
  • Enum and API surface updates: old generic enums (AuthMethod, ServiceId, etc.) are replaced with Project-prefixed equivalents; Projects.create() drops legal/branding fields and Projects.get() is removed in favour of Project.get().
  • New types and services: Presence, PresenceList, InsightList, ReportList models added; BackupDatabase policy/archive fields corrected; File.sizeActual added.

Confidence Score: 3/5

Safe to merge with caution — the concurrent upload logic works correctly in the happy path, but has an edge case where a chunk failure leaves orphaned server-side writes.

The concurrent pool correctly serialises shared-counter updates thanks to JavaScript’s single-threaded event loop, and progress reporting is accurate. However, when any chunk rejects the outer promise, the pool’s bookkeeping leaves server-side upload state in an indeterminate position that can affect retries.

src/client.ts — the new concurrent chunk upload pool in chunkedUpload()

Important Files Changed

Filename Overview
src/client.ts Core change: sequential chunk upload loop replaced with a concurrent pool (CONCURRENCY=8). After failure, queued chunks continue to be dispatched; dead-code early-return also present.
src/channel.ts Adds Presence channel type and static helpers; straightforward extension of the existing pattern.
src/services/project.ts Enum imports renamed; new get() and updateDenyAliasedEmailPolicy() methods added; generated code, no logic issues.
src/services/projects.ts create() signature trimmed; get() removed (moved to Project service). Generated code, no logic issues.
src/models.ts Adds Presence, PresenceList, InsightList, ReportList model types; fixes BackupDatabase field types; adds File.sizeActual.
src/index.ts Re-exports new services and renamed enums; removes old enum exports. Correctly reflects the rest of the PR's changes.

Comments Outside Diff (1)

  1. src/client.ts, line 1171-1197 (link)

    P1 New chunks dispatched after upload failure

    When a chunk fails and reject is called, the already-in-flight chunks continue to run and their .then() callbacks call uploadNext(). Because completed is never incremented for the failing chunk, completed === chunks.length is never true inside uploadNext(), so the while loop keeps firing and dispatches any remaining queued chunks. In practice this means: a failure mid-way through an 8-chunk file (CONCURRENCY=8, 10 total chunks) will still trigger uploads of chunks 8 and 9 after rejection. The outer await surfaces the error correctly, but the server receives orphaned writes under the failed uploadId— which can advance the server-side upload state unexpectedly and make a subsequent retry behave differently than a clean start.

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

Comment thread src/client.ts
Comment on lines +1003 to +1007
if (totalChunks === 1) {
return response;
}

// Prepare remaining chunks

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Dead code: totalChunks === 1 can never be reached

The guard at line 977 already returns early whenever file.size <= Client.CHUNK_SIZE, which is exactly the condition that makes Math.ceil(file.size / CHUNK_SIZE) === 1. So by the time execution reaches this check, totalChunks is always ≥ 2 and this branch is unreachable. Remove it to avoid misleading future readers.

Suggested change
if (totalChunks === 1) {
return response;
}
// Prepare remaining chunks
// Prepare remaining chunks

@TorstenDittmann

Copy link
Copy Markdown
Contributor Author

Closing in favor of focused PR #96.

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