Skip to content

fix(downloads): aggregate concurrent model download progress in the macOS Dock - #1422

Open
xwsjjctz wants to merge 2 commits into
Comfy-Org:mainfrom
xwsjjctz:fix/aggregate-download-taskbar-progress
Open

fix(downloads): aggregate concurrent model download progress in the macOS Dock#1422
xwsjjctz wants to merge 2 commits into
Comfy-Org:mainfrom
xwsjjctz:fix/aggregate-download-taskbar-progress

Conversation

@xwsjjctz

Copy link
Copy Markdown

Summary

  • Aggregate concurrent model downloads into one byte-weighted Dock progress value on macOS.
  • Preserve native per-window taskbar progress behavior on other platforms.
  • Handle downloads with unknown sizes, completed jobs, cancellations, failures, and detached windows.
  • Add regression coverage for concurrent progress aggregation and cancellation.

Problem

Each model download currently reports its own progress directly to Electron's setProgressBar() API.

On macOS, the application has one shared Dock progress indicator. Concurrent downloads therefore overwrite each other's values, causing the Dock progress to jump forwards and backwards instead of representing the complete download workload.

Fixes #1421.

Root Cause

Taskbar progress was tracked as a property of an individual download update. There was no persistent batch keyed by download ID and no aggregation across windows on macOS.

As a result, whichever download reported most recently controlled the Dock indicator.

Changes

  • Track taskbar download state by stable download ID.
  • Aggregate macOS Dock progress across all live application windows.
  • Calculate aggregate progress using total received bytes divided by total expected bytes.
  • Keep completed downloads in the batch until the remaining downloads finish, preventing the aggregate from jumping backwards.
  • Remove cancelled and failed downloads immediately and recalculate progress from the remaining workload.
  • Keep the Dock progress empty while an active download has not reported its total size, avoiding a brief full-progress flash on macOS.
  • Clear the native progress indicator only after the active batch finishes.
  • Remove taskbar state when its owner window is detached or destroyed.
  • Retain per-window progress behavior on non-macOS platforms.

Validation

Automated

  • pnpm exec vitest run src/main/lib/comfyDownloadManagerModelJobs.test.ts
    • 61 tests passed
  • pnpm test
    • 255 test files passed
    • 4,343 tests passed
    • 2 existing tests skipped
  • pnpm run typecheck
  • pnpm run lint
  • pnpm run format:check
  • pnpm run build

Manual macOS Testing

Tested with a locally packaged Comfy Desktop 1.0.39 build on macOS 15.7.7, Apple Silicon (arm64).

  1. Started multiple large model downloads concurrently with different file sizes and download speeds.
  2. Compared the individual percentages in the download list with the progress displayed on the Dock icon.
  3. Verified that concurrent downloads are represented by one aggregate Dock progress value instead of alternating between individual percentages.
  4. During the first validation pass, identified two aggregation edge cases:
    • Starting a download could briefly display a full Dock progress bar before its total size was known.
    • Cancelling one concurrent download did not immediately update the aggregate.
  5. After addressing both edge cases, repeated the same concurrent-download scenario.
  6. Verified that the Dock no longer flashes as complete when a download starts.
  7. Cancelled one download while another remained active and verified that the Dock progress was recalculated from the remaining workload.
  8. Continued downloading multiple models and observed normal aggregate progress behavior.

Compatibility

This change does not modify public APIs, download identifiers, file layout, or model download behavior. Non-macOS platforms retain per-window native taskbar progress.

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9395be3f-bd67-430e-b6c2-be38b36201b6

📥 Commits

Reviewing files that changed from the base of the PR and between 1ff443a and 920e72c.

📒 Files selected for processing (2)
  • src/main/lib/comfyDownloadManager.ts
  • src/main/lib/comfyDownloadManagerModelJobs.test.ts

Included review availability: Your plan includes up to 2 reviews per rolling hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The taskbar download progress now aggregates concurrent downloads by byte count. Completed jobs remain represented until the batch finishes, while failed or cancelled jobs are removed. Window detachment clears related tracking and progress.

Changes

Taskbar progress aggregation

Layer / File(s) Summary
Track and clean up aggregate progress
src/main/lib/comfyDownloadManager.ts
Per-window tracking aggregates download bytes, retains completed jobs, removes failed or cancelled jobs, handles macOS application-wide progress, and clears detached windows.
Validate download lifecycle aggregation
src/main/lib/comfyDownloadManagerModelJobs.test.ts
Tests cover byte-weighted concurrent progress, incomplete sizes, completed-job retention, final clearing, and cancellation handling.

Possibly related PRs

Suggested reviewers: kosinkadink, shrimbly, maanilverma

Merge Risk: ⚪ Minimal · up to 920e7

The PR aggregates concurrent model-download progress into one byte-weighted macOS Dock indicator while preserving per-window behavior elsewhere; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes implement byte-weighted aggregate Dock progress and handle starts, completions, failures, cancellations, and window lifecycle states required by issue #1421.
Out of Scope Changes check ✅ Passed The changes are limited to download manager progress handling and focused regression tests for the linked macOS Dock progress issue.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

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.

@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

✅ All contributors have signed the CLA. Thank you! This PR is ready to be merged.
Posted by the CLA Assistant Lite bot.

@xwsjjctz xwsjjctz changed the title Fix macOS Dock progress for concurrent model downloads fix(downloads): aggregate concurrent model download progress in the macOS Dock Aug 17, 2026
@xwsjjctz
xwsjjctz marked this pull request as ready for review August 17, 2026 21:09
@xwsjjctz

Copy link
Copy Markdown
Author

I have read and agree to the Contributor License Agreement

@xwsjjctz

Copy link
Copy Markdown
Author

Hi @Kosinkadink, just a friendly follow-up on this PR. Thanks for your response in #1421. When you have a chance, could you please take a look? The fix is ready for review and includes focused regression tests and manual verification on a packaged macOS build. I would be happy to address any feedback. Thank you!

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.

[macOS] Dock progress jumps between concurrent model downloads instead of showing aggregate progress

1 participant