Skip to content

Jb/streaming assembly utkarsh#1219

Merged
utkarshdalal merged 8 commits intomasterfrom
jb/streaming-assembly-utkarsh
Apr 14, 2026
Merged

Jb/streaming assembly utkarsh#1219
utkarshdalal merged 8 commits intomasterfrom
jb/streaming-assembly-utkarsh

Conversation

@utkarshdalal
Copy link
Copy Markdown
Owner

@utkarshdalal utkarshdalal commented Apr 14, 2026

Description

Recording

Type of Change

  • Bug fix
  • Performance / stability improvement
  • Compatibility improvements
  • Other (requires prior approval)

Checklist

  • If I have access to #code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.
  • This change aligns with the current project scope (core functionality, stability, or performance). If not, it has been explicitly approved beforehand.
  • I have attached a recording of the change.
  • I have read and agree to the contribution guidelines in CONTRIBUTING.md.

Summary by cubic

Switches Epic and GOG installs to streaming download+assembly to cut peak disk usage to ~1x and speed up resumes. Adds CDN ranking and smoother, byte-based progress for GOG, plus one‑click resume from the game screen.

  • New Features

    • Streaming download+assembly for Epic/GOG (incl. DLC/dependencies): file‑ordered chunk queue, on‑the‑fly assembly, immediate chunk cleanup, and zero‑chunk file handling.
    • GOG: rank CDN bases by live probe; ordered URL candidates with auto refresh; time‑based byte progress; one‑click resume from the game screen.
  • Refactors and Performance

    • Shared StreamingAssembly with unit tests; unified streaming loop in EpicDownloadManager/GOGDownloadManager; parallelism via PrefManager.downloadSpeed and Net.httpForParallelDownloads.
    • GOG: safer chunk writes with .part + MD5 verify; total expected bytes include resumed data; better cancellation propagation.
    • GOGManifestParser builds ordered URL candidate lists (keeps back‑compat helpers); UI tweaks: resume logic in GOGAppScreen and single‑line percent label in LibraryAppScreen.

Written for commit c0eaa24. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • Streaming download pipeline: files begin assembling as soon as their parts are available.
    • Dynamic per-download parallelism tuned to connection speed.
    • New network client mode optimized for parallel downloads.
  • Improvements

    • Ordered per-chunk URL fallbacks and refreshed retry behavior for more reliable downloads.
    • Safer on-disk streaming, incremental hashing, and chunk cache cleanup after last use.
    • More granular, real-time progress/status reporting and throttled progress events.
    • Install button now resumes partial downloads immediately.
  • Bug Fixes

    • Fixed download percentage text wrapping in the library UI.
  • Tests

    • Added comprehensive tests for streaming assembly, readiness, and deletion semantics.

jeremybernstein and others added 4 commits April 11, 2026 17:36
replace two-phase (download all chunks → assemble all files) with a
unified loop that assembles files front-to-back as their chunks land
and deletes consumed chunks immediately. peak disk usage drops from
~2x install size to ~1x.

- StreamingAssembly: shared pure logic for chunk ordering, last-file
  tracking, readiness checks, and safe deletion decisions
- EpicDownloadManager: downloadAndAssembleEpicChunks replaces separate
  download+assembly phases for both base game and DLC
- GOGDownloadManager: downloadAndAssembleChunks with secure link
  refresh, used by both main game and dependency downloads. removed
  dead downloadChunksSimple and assembleFiles.
- 14 unit tests covering ordering, deduplication, shared chunks,
  cleanup safety, and full batch-loop simulations
mirrors the GOG fix — when all files have zero chunks, the chunk loop
never executes and assembly never runs without a trailing pass.
… better, made downloading UI progress smoother
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 14, 2026

📝 Walkthrough

Walkthrough

Adds a streaming download-and-assembly pipeline: new StreamingAssembly utilities for chunk ordering and last-consumer logic, rewrites Epic/GOG download managers to download-and-assemble chunks incrementally with dynamic parallelism and per-chunk URL candidates, adds an OkHttp factory for parallel downloads, small UI/resume tweaks, and unit tests.

Changes

Cohort / File(s) Summary
Streaming Pipeline Core
app/src/main/java/app/gamenative/service/StreamingAssembly.kt
New utility object with: ordered chunk queue, chunk->last-file map, ready-file counter, and per-file deletion selector.
Epic Download Manager
app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt
Replaced fixed batching with streaming download+assemble flow, dynamic parallelism from PrefManager, streaming progress/status, and chunk deletion after last consumer.
GOG Download Manager
app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt
Rewrote to streaming download+assemble loop, improved resume byte accounting, per-chunk ordered URL candidates and probing, streaming-to-*.part with atomic finalize, throttled progress, and merged dependency downloads.
GOG Manifest URL Handling
app/src/main/java/app/gamenative/service/gog/api/GOGManifestParser.kt
Now produces ordered per-chunk URL candidate lists; compatibility single-URL wrappers retained; product-aware candidate builder added.
Network Utilities
app/src/main/java/app/gamenative/utils/NetworkUtils.kt
Added Net.httpForParallelDownloads(parallelDownloads: Int) returning an OkHttpClient with a dispatcher sized for parallel download concurrency.
UI: Library / GOG App Screen
app/src/main/java/app/gamenative/ui/screen/library/...
Constrain download percent text to one line; click handlers resume partial downloads immediately; removed some debug logs.
Tests
app/src/test/java/app/gamenative/service/StreamingAssemblyTest.kt
New unit tests covering ordered queue, last-file mapping, readiness counting, deletion rules, and end-to-end assembly simulations.

Sequence Diagram(s)

sequenceDiagram
  participant Manager as DownloadManager
  participant Net as Net.httpClient
  participant Disk as ChunkStorage
  participant Assembler as FileAssembler
  participant UI as ProgressReporter

  Manager->>Net: request batch of chunk URL candidates
  Net-->>Manager: stream chunk data (progress events)
  Net->>Disk: write chunk.part (compute MD5 while streaming)
  Disk-->>Net: confirm write + MD5
  Manager->>Assembler: notify chunk available
  Assembler->>Manager: check readiness for nextFileIndex (StreamingAssembly.countReadyFiles)
  alt file ready
    Manager->>Assembler: assemble file from available chunks
    Assembler->>Disk: read chunks, write file
    Assembler->>Manager: file assembled (nextFileIndex++)
    Manager->>Disk: delete chunks where lastConsumer == assembledFileIndex
    Manager->>UI: update (downloadedChunks/total, nextFile/total)
  else not ready
    Manager->>UI: update progress only
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~55 minutes

Possibly related issues

Possibly related PRs

Suggested Reviewers

  • phobos665

🐰
I queued the chunks in neat first-glow,
Nibble by nibble the files will grow,
I track the last bite, then toss the rind,
Hopping happy — stream and bind! 🥕✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Jb/streaming assembly utkarsh' is vague and uses non-descriptive terms like a branch name and developer identifier rather than clearly summarizing the main change. Revise title to clearly describe the main change, e.g., 'Switch to streaming download+assembly for Epic and GOG installs' or 'Implement streaming download+assembly pipeline to reduce peak disk usage'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The PR description includes the required checklist items checked, identifies it as a performance/stability improvement, and provides a detailed summary of changes via the cubic auto-generated description.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jb/streaming-assembly-utkarsh

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

# Conflicts:
#	app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt
#	app/src/main/java/app/gamenative/service/gog/api/GOGManifestParser.kt
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt (1)

904-945: Epic still assembles at batch boundaries, not on chunk completion.

awaitAll() holds assembly until the slowest chunk in the batch finishes, so ready files cannot be assembled and last-use chunks cannot be deleted as soon as they arrive. Mirroring the per-completion channel loop used in GOGDownloadManager.downloadAndAssembleChunks(...) would preserve more of the streaming disk-usage win here.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt` around
lines 904 - 945, The current loop waits on chunkBatch.map { ... }.awaitAll(),
which delays assembly until the slowest chunk in the batch completes; change the
logic to process each chunk as it completes (like
GOGDownloadManager.downloadAndAssembleChunks) by launching async downloads for
each chunk (using downloadChunkWithRetry) and consuming their Deferred results
via a completion channel or select so you can immediately mark
downloadedChunkIds, attempt assembleFile for any files whose chunkParts are now
all present, and delete last-use chunk files (using chunkLastFile and
chunkCacheDir) as soon as their last file is assembled; ensure you preserve
progress/updateStatusMessage and the same failure handling by short-circuiting
on the first failed Deferred (returning a Result.failure with the same
exceptions).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt`:
- Around line 893-908: The Epic download loop uses the default OkHttp dispatcher
which limits same-host concurrency; replace the current OkHttp client used by
Epic downloads with Net.httpForParallelDownloads(parallelDownloads) (where
parallelDownloads is computed from PrefManager.downloadSpeed) so the dispatcher
is tuned for the actual parallelism. Locate where the Epic HTTP
client/okHttpClient is created or passed into downloadChunkWithRetry and
instantiate or inject the client via
Net.httpForParallelDownloads(parallelDownloads), then ensure
downloadChunkWithRetry and any callers (e.g., the chunk download loop over
chunkQueue and functions using cdnUrls) use that client instance so
parallelDownloads is effective.

In `@app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt`:
- Around line 880-900: The retry path after refreshSecureLinks currently obtains
retryResults but does not update downloadedChunkIds or trigger assembleReady(),
so chunks that succeeded on retry remain unrecorded; update the logic after
retryResults are awaited to iterate retryResults (associated with chunkMd5 keys
from chunkBatch/currentChunkUrlCandidates), collect successfully downloaded
chunk IDs and merge them into downloadedChunkIds (same structure/population used
by the initial batch), and then invoke the same assembleReady() /
assembly-triggering flow (or re-run the existing assembleReady check) so retried
successes are counted and assembly can complete; touch functions/variables:
expiredLinkFailures, refreshSecureLinks(...), currentChunkUrlCandidates,
retryResults, downloadChunkWithRetry(...), downloadedChunkIds, and
assembleReady() to ensure parity with the initial-success handling.

---

Nitpick comments:
In `@app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt`:
- Around line 904-945: The current loop waits on chunkBatch.map { ...
}.awaitAll(), which delays assembly until the slowest chunk in the batch
completes; change the logic to process each chunk as it completes (like
GOGDownloadManager.downloadAndAssembleChunks) by launching async downloads for
each chunk (using downloadChunkWithRetry) and consuming their Deferred results
via a completion channel or select so you can immediately mark
downloadedChunkIds, attempt assembleFile for any files whose chunkParts are now
all present, and delete last-use chunk files (using chunkLastFile and
chunkCacheDir) as soon as their last file is assembled; ensure you preserve
progress/updateStatusMessage and the same failure handling by short-circuiting
on the first failed Deferred (returning a Result.failure with the same
exceptions).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8014e270-0ca6-4c43-ba7f-85e81dec7c56

📥 Commits

Reviewing files that changed from the base of the PR and between 22cf4c5 and 86b0c5e.

📒 Files selected for processing (8)
  • app/src/main/java/app/gamenative/service/StreamingAssembly.kt
  • app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt
  • app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt
  • app/src/main/java/app/gamenative/service/gog/api/GOGManifestParser.kt
  • app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt
  • app/src/main/java/app/gamenative/ui/screen/library/appscreen/GOGAppScreen.kt
  • app/src/main/java/app/gamenative/utils/NetworkUtils.kt
  • app/src/test/java/app/gamenative/service/StreamingAssemblyTest.kt

Comment on lines +893 to +908
val parallelDownloads = PrefManager.downloadSpeed.coerceAtLeast(1)
val downloadedChunkIds = mutableSetOf<String>()
var nextFileToAssemble = 0

downloadInfo.setProgress(0.0f)

for (chunkBatch in chunkQueue.chunked(parallelDownloads)) {
if (!downloadInfo.isActive()) {
return@withContext Result.failure(Exception("Download cancelled"))
}

val results = chunkBatch.map { chunk ->
async {
downloadChunkWithRetry(chunk, chunkCacheDir, chunkDir, cdnUrls, downloadInfo)
}
}.awaitAll()
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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

What are the default values of OkHttp Dispatcher.maxRequestsandDispatcher.maxRequestsPerHost, and do they cap concurrent requests to the same host when a custom dispatcher is not configured?

💡 Result:

The default values of OkHttp Dispatcher's maxRequests and maxRequestsPerHost are 64 and 5, respectively. Yes, these values cap concurrent requests to the same host when a custom dispatcher is not configured, as the Dispatcher is used by default in OkHttpClient and enforces these limits by checking running calls per host before promoting new async calls to execution.

Citations:


🏁 Script executed:

# Find okHttpClient instantiation in EpicDownloadManager
rg -A 5 "okHttpClient" app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt

Repository: utkarshdalal/GameNative

Length of output: 767


🏁 Script executed:

# Search for Net.httpForParallelDownloads to see what it does
find . -type f -name "*.kt" | head -20 | xargs rg -l "httpForParallelDownloads"

Repository: utkarshdalal/GameNative

Length of output: 51


🏁 Script executed:

# Search for the class definition and dispatcher setup
rg -B 5 -A 10 "class EpicDownloadManager" app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt | head -30

Repository: utkarshdalal/GameNative

Length of output: 671


🏁 Script executed:

# Search for Net class and httpForParallelDownloads function
rg -r "httpForParallelDownloads" --type kt

Repository: utkarshdalal/GameNative

Length of output: 463


🏁 Script executed:

# Look for Net class definition
find . -type f -name "*.kt" | xargs rg -l "object Net|class Net" | head -5

Repository: utkarshdalal/GameNative

Length of output: 228


🏁 Script executed:

# Search in GOGDownloadManager for dispatcher configuration
rg -B 5 -A 5 "Dispatcher\|httpForParallelDownloads" app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt | head -40

Repository: utkarshdalal/GameNative

Length of output: 49


🏁 Script executed:

# Read NetworkUtils.kt to find httpForParallelDownloads
cat app/src/main/java/app/gamenative/utils/NetworkUtils.kt

Repository: utkarshdalal/GameNative

Length of output: 1997


🏁 Script executed:

# Search GOG more carefully for any dispatcher config
rg "Dispatcher" app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt

Repository: utkarshdalal/GameNative

Length of output: 722


🏁 Script executed:

# Check if GOG uses Net.httpForParallelDownloads
rg "Net\.httpForParallelDownloads\|httpForParallelDownloads" app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt

Repository: utkarshdalal/GameNative

Length of output: 49


🏁 Script executed:

# Check how Epic constructs chunk URLs - are they same host?
rg -B 3 -A 3 "cdnUrls" app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt | head -30

Repository: utkarshdalal/GameNative

Length of output: 1386


🏁 Script executed:

# Check GOG's okHttpClient setup
rg -B 5 -A 10 "private val okHttpClient" app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt

Repository: utkarshdalal/GameNative

Length of output: 49


🏁 Script executed:

# Check if GOG uses Net.http or creates its own
rg "okHttpClient\|Net\.http" app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt | head -20

Repository: utkarshdalal/GameNative

Length of output: 49


🏁 Script executed:

# Search for downloadChunk method in GOG
rg -B 2 -A 10 "fun downloadChunk" app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt | head -40

Repository: utkarshdalal/GameNative

Length of output: 978


🏁 Script executed:

# Check GOG imports to see what it uses for HTTP
head -30 app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt

Repository: utkarshdalal/GameNative

Length of output: 1180


🏁 Script executed:

# Find where GOG's downloadChunk methods are called to see what httpClient is passed
rg -B 5 "downloadChunkWithRetry\|downloadChunk" app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt | grep -A 5 "downloadChunk\|httpClient\|Net\|parallelDownloads" | head -50

Repository: utkarshdalal/GameNative

Length of output: 49


🏁 Script executed:

# Look for the main download orchestration in GOG to see how httpClient is created/passed
rg -B 3 -A 10 "fun download.*Result" app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt | head -80

Repository: utkarshdalal/GameNative

Length of output: 571


🏁 Script executed:

# Find the main download function in GOG
rg -B 2 -A 20 "suspend fun download" app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt | head -100

Repository: utkarshdalal/GameNative

Length of output: 4396


🏁 Script executed:

# Search for where Net.httpForParallelDownloads is actually called in the codebase
rg -B 3 -A 3 "Net\.httpForParallelDownloads" app/src/main/java/app/gamenative/service/gog/

Repository: utkarshdalal/GameNative

Length of output: 980


Use Net.httpForParallelDownloads(parallelDownloads) for the Epic HTTP client.

The current okHttpClient uses OkHttp's default dispatcher, which caps same-host concurrency to 5 requests. With parallelDownloads potentially much higher, this becomes a bottleneck. GOG already uses Net.httpForParallelDownloads() to tune the dispatcher—apply the same pattern to Epic to make the parallelism setting effective.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt` around
lines 893 - 908, The Epic download loop uses the default OkHttp dispatcher which
limits same-host concurrency; replace the current OkHttp client used by Epic
downloads with Net.httpForParallelDownloads(parallelDownloads) (where
parallelDownloads is computed from PrefManager.downloadSpeed) so the dispatcher
is tuned for the actual parallelism. Locate where the Epic HTTP
client/okHttpClient is created or passed into downloadChunkWithRetry and
instantiate or inject the client via
Net.httpForParallelDownloads(parallelDownloads), then ensure
downloadChunkWithRetry and any callers (e.g., the chunk download loop over
chunkQueue and functions using cdnUrls) use that client instance so
parallelDownloads is effective.

Comment thread app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt`:
- Around line 1256-1261: The debug log prints full signed CDN URLs
(ranked.joinToString()) which exposes bearer tokens; update the logging in
GOGDownloadManager (the block that computes `ranked` from `scored` and calls
`Timber.tag("GOG").d(...)`) to redact query parameters before logging—e.g., map
each URL in `ranked` to a sanitized form by stripping or replacing the query
string (take substring before '?' or parse via URI and omit getQuery()) and then
join those sanitized entries for the log message instead of the raw
`ranked.joinToString()`.
- Around line 893-917: The current refresh+retry logic replays the whole
chunkBatch via chunkBatch.map { async { ... downloadChunkWithRetry(...) } }
which re-downloads already-consumed chunks; change it to only retry failed chunk
IDs: after awaiting the initial batch, compute failedChunkIds =
retryResults.zip(chunkBatch).filter { it.first.isFailure }.map { corresponding
chunkMd5 } (use retryResults and chunkBatch to correlate), then only call
downloadChunkWithRetry for those failedChunkIds (using currentChunkUrlCandidates
and downloadChunkWithRetry) and only add successfully retried IDs to
downloadedChunkIds (do not re-add already-successful chunks); keep the existing
assembleReady() and progress update but derive progress from
downloadedChunkIds.size so bytesDownloaded and immediate deletion behave
correctly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 455e9b19-df89-43e9-bc32-9b205f9dd0ea

📥 Commits

Reviewing files that changed from the base of the PR and between 86b0c5e and 710ec54.

📒 Files selected for processing (1)
  • app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt

Comment thread app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt Outdated
Comment thread app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt
Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt">

<violation number="1" location="app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt:894">
P1: Retrying only expired-link chunks drops other failures in the same batch. If one chunk needs a secure-link refresh and another failed for a different reason, the second failure is no longer retried or reported here, so the install can continue until it later fails with a generic incomplete-assembly error.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt`:
- Around line 880-930: The code currently ignores non-expired failures when
expiredLinkFailures triggers a secure-link refresh; before calling
refreshSecureLinks (and likewise after retryResults), check for any failures in
results whose chunkMd5 is not in expiredLinkFailures (i.e., non-expired
failures) and return@withContext Result.failure(...) immediately; specifically,
compute nonExpiredFailures from results by filtering isFailure and chunkMd5 not
in expiredLinkFailures.map { ... }, and if nonExpiredFailures.isNotEmpty()
return the first failure's exception (or a wrapped Exception) so that
refreshSecureLinks and retry logic (refreshSecureLinks, retryResults,
failedChunkIds, downloadChunkWithRetry, assembleReady) are skipped when there
are non-expired errors.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: df777a02-7d45-4c19-853b-36dd2700534a

📥 Commits

Reviewing files that changed from the base of the PR and between 710ec54 and a42e038.

📒 Files selected for processing (1)
  • app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt

Comment thread app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt (1)

1246-1273: Consider parallel CDN probing for faster startup.

The sequential HEAD probes work correctly, but for N CDN endpoints, startup latency is O(N × avg_probe_time). Parallel probing would reduce this to O(max_probe_time).

♻️ Optional: Parallel probe implementation
 private suspend fun rankBaseUrlsByProbe(baseUrls: List<String>, label: String): List<String> = withContext(Dispatchers.IO) {
     val urls = baseUrls.map { it.trim() }.filter { it.isNotEmpty() }.distinct()
     if (urls.size <= 1) return@withContext urls

-    val scored = urls.map { url ->
+    val scored = urls.map { url ->
+        async {
             val start = System.nanoTime()
             val success = try {
                 val request = Request.Builder()
                     .url(url)
                     .head()
                     .header("User-Agent", "GOG Galaxy")
                     .build()
                 Net.http.newCall(request).execute().use { response ->
                     response.code in 200..499
                 }
             } catch (_: Exception) {
                 false
             }
             val elapsedMs = (System.nanoTime() - start) / 1_000_000
             Triple(url, success, elapsedMs)
+        }
-    }
+    }.awaitAll()

     val ranked = scored
         .sortedWith(compareByDescending<Triple<String, Boolean, Long>> { it.second }.thenBy { it.third })
         .map { it.first }

     ranked
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt` around
lines 1246 - 1273, rankBaseUrlsByProbe currently probes CDN baseUrls
sequentially causing O(N) startup latency; change it to run probes in parallel
by launching coroutines for each url (e.g., inside coroutineScope { ... } using
async(Dispatchers.IO) for each url) so each probe measures success and elapsedMs
concurrently, collect all Deferred results with awaitAll(), then sort by success
desc then elapsedMs asc as before and return the ordered url list; ensure each
probe still uses the same Request/Net.http call and catches exceptions per-probe
to produce (url, success, elapsedMs).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt`:
- Around line 1246-1273: rankBaseUrlsByProbe currently probes CDN baseUrls
sequentially causing O(N) startup latency; change it to run probes in parallel
by launching coroutines for each url (e.g., inside coroutineScope { ... } using
async(Dispatchers.IO) for each url) so each probe measures success and elapsedMs
concurrently, collect all Deferred results with awaitAll(), then sort by success
desc then elapsedMs asc as before and return the ordered url list; ensure each
probe still uses the same Request/Net.http call and catches exceptions per-probe
to produce (url, success, elapsedMs).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 10e8d926-353e-4d53-a24e-378447090426

📥 Commits

Reviewing files that changed from the base of the PR and between a42e038 and c0eaa24.

📒 Files selected for processing (1)
  • app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt

@utkarshdalal utkarshdalal merged commit 979c278 into master Apr 14, 2026
3 checks passed
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