feat: Add cancel control to in-flight model downloads#60
Merged
Conversation
Wires Swift Task cancellation through to URLSession so users can stop
a long model download from the UI. Until now, Task.cancel() only flipped
Task.isCancelled — the URLSession download kept running until natural
completion, wasting bytes and silently ignoring the user's intent.
Changes:
- ModelDownloadManager.cancel(filename:) — public, idempotent.
- ModelDownloadSessionDelegate.download(from:) wraps the continuation in
withTaskCancellationHandler. The onCancel block calls .cancel() on the
retained URLSessionDownloadTask, which fires didCompleteWithError with
URLError.cancelled and resumes the continuation.
- New pure helper DownloadOutcomeClassifier distinguishes user-cancel
(CancellationError or URLError.cancelled) from real failures, so
performDownload's catch routes the two correctly:
- cancel -> restore .idle (or .downloaded if a prior copy is on disk)
- failure -> .failed(message), shown to the user
- DownloadableModelRow renders an xmark.circle.fill cancel button next
to the percentage during .downloading; tooltip explains.
Tests (7 new, 32 total):
- DownloadOutcomeClassifierTests covers cancel surfaces (CancellationError,
URLError.cancelled), real failure surfaces (timedOut, notConnected,
badServerResponse, generic NSError), and the LlamaRuntimeError case
(must NOT be misclassified as cancel — would silently roll back a real
failure to .idle).
Verified locally:
xcodebuild test -project tabby.xcodeproj -scheme tabby \
-destination 'platform=macOS' CODE_SIGNING_ALLOWED=NO
# ** TEST SUCCEEDED ** 32 tests 0 failures
First of three stacked PRs against #6:
1. (this PR) cancel + cleanup
2. size + SHA-256 validation
3. retry UX polish
Refs #6
Owner
|
Next time can you attach a screenshot? |
Owner
|
@copilot resolve the merge conflicts in this pull request |
…ancel # Conflicts: # tabby.xcodeproj/project.pbxproj # tabby/Services/Utilities/ModelDownloadManager.swift Co-authored-by: FuJacob <141651335+FuJacob@users.noreply.github.com>
Here's the screenshot showing the cancel button (×) next to the download progress:
The cancel control renders inline at 32% progress, showing the percent and a dismiss button on the right.
Resolved in commit
|
FuJacob
added a commit
that referenced
this pull request
Apr 28, 2026
* feat: Add cancel control to in-flight model downloads (#6) Wires Swift Task cancellation through to URLSession so users can stop a long model download from the UI. Until now, Task.cancel() only flipped Task.isCancelled — the URLSession download kept running until natural completion, wasting bytes and silently ignoring the user's intent. Changes: - ModelDownloadManager.cancel(filename:) — public, idempotent. - ModelDownloadSessionDelegate.download(from:) wraps the continuation in withTaskCancellationHandler. The onCancel block calls .cancel() on the retained URLSessionDownloadTask, which fires didCompleteWithError with URLError.cancelled and resumes the continuation. - New pure helper DownloadOutcomeClassifier distinguishes user-cancel (CancellationError or URLError.cancelled) from real failures, so performDownload's catch routes the two correctly: - cancel -> restore .idle (or .downloaded if a prior copy is on disk) - failure -> .failed(message), shown to the user - DownloadableModelRow renders an xmark.circle.fill cancel button next to the percentage during .downloading; tooltip explains. Tests (7 new, 32 total): - DownloadOutcomeClassifierTests covers cancel surfaces (CancellationError, URLError.cancelled), real failure surfaces (timedOut, notConnected, badServerResponse, generic NSError), and the LlamaRuntimeError case (must NOT be misclassified as cancel — would silently roll back a real failure to .idle). Verified locally: xcodebuild test -project tabby.xcodeproj -scheme tabby \ -destination 'platform=macOS' CODE_SIGNING_ALLOWED=NO # ** TEST SUCCEEDED ** 32 tests 0 failures First of three stacked PRs against #6: 1. (this PR) cancel + cleanup 2. size + SHA-256 validation 3. retry UX polish Refs #6 * feat: Validate model downloads against expected size and SHA-256 (#6) Adds integrity checks for curated model downloads. After a download finishes, the file is moved to a staging URL, validated against expected size + SHA-256, and only then swapped into the install location. If validation fails, the staged file is removed and the prior install (if any) stays in place — a corrupt download can't take out a working copy. Validation strategy: both size and SHA-256. - Size is a fast pre-check that catches truncated downloads. - SHA-256 is the integrity guarantee against silent corruption (bit flips, partial writes, mirror substitution). - Either alone is insufficient; together they're cheap. Catalog now carries `expectedSizeBytes` and `sha256` for the three curated models. Values were captured from HuggingFace's `x-linked-size` and `x-linked-etag` response headers — refresh procedure documented in the catalog. Both fields are Optional; validators no-op when nil so future catalog additions don't have to wait on metadata to land. Streams the SHA-256 in 1 MB chunks so the largest curated model (~4.5 GB) doesn't get loaded entirely into memory. Tests (11 new, 43 total): - validateSize: match, mismatch (both directions), nil expected, missing file - validateSHA256: known-good hash, uppercase tolerance, mismatch, nil expected, missing file - streaming guard: 3 MB fixture exercises the multi-chunk read loop so a regression to single-shot read would fail the test Verified locally: xcodebuild test -project tabby.xcodeproj -scheme tabby \ -destination 'platform=macOS' CODE_SIGNING_ALLOWED=NO # ** TEST SUCCEEDED ** 43 tests 0 failures Stacked on #60 (download cancel). Folds into #6. Refs #6 * feat: Make download failure messages legible in the model catalog (#6) Failure messages now have their own wrapping row below the row body, with a warning icon. Validation errors include exact byte counts and partial checksum prefixes — fitting them inline with the size label clipped them on smaller windows. Changes: - New `failureMessageRow(message:)` renders the localized error in red with `exclamationmark.triangle.fill`. Pinned to fixedSize(vertical:) so SwiftUI allocates space for the wrap rather than truncating. - Metadata text in `.failed` state now shows "Download failed" instead of the full statusText — the full message lives in the dedicated row. - Retry button gets `arrow.counterclockwise` symbol; pairs with the warning row to reinforce "something went wrong, try again." No new tests — pure SwiftUI tweak. Verified the row still builds: xcodebuild build -project tabby.xcodeproj -scheme tabby \ -configuration Debug -destination 'platform=macOS' \ CODE_SIGNING_ALLOWED=NO # ** BUILD SUCCEEDED ** And the existing 43 tests still pass. Stacked on #61 (validation) and #60 (cancel). Closes the last acceptance criterion of #6 — failed downloads can be retried from the same UI with a legible error. Refs #6 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: FuJacob <141651335+FuJacob@users.noreply.github.com>
FuJacob
added a commit
that referenced
this pull request
Apr 28, 2026
* feat: Add cancel control to in-flight model downloads (#6) Wires Swift Task cancellation through to URLSession so users can stop a long model download from the UI. Until now, Task.cancel() only flipped Task.isCancelled — the URLSession download kept running until natural completion, wasting bytes and silently ignoring the user's intent. Changes: - ModelDownloadManager.cancel(filename:) — public, idempotent. - ModelDownloadSessionDelegate.download(from:) wraps the continuation in withTaskCancellationHandler. The onCancel block calls .cancel() on the retained URLSessionDownloadTask, which fires didCompleteWithError with URLError.cancelled and resumes the continuation. - New pure helper DownloadOutcomeClassifier distinguishes user-cancel (CancellationError or URLError.cancelled) from real failures, so performDownload's catch routes the two correctly: - cancel -> restore .idle (or .downloaded if a prior copy is on disk) - failure -> .failed(message), shown to the user - DownloadableModelRow renders an xmark.circle.fill cancel button next to the percentage during .downloading; tooltip explains. Tests (7 new, 32 total): - DownloadOutcomeClassifierTests covers cancel surfaces (CancellationError, URLError.cancelled), real failure surfaces (timedOut, notConnected, badServerResponse, generic NSError), and the LlamaRuntimeError case (must NOT be misclassified as cancel — would silently roll back a real failure to .idle). Verified locally: xcodebuild test -project tabby.xcodeproj -scheme tabby \ -destination 'platform=macOS' CODE_SIGNING_ALLOWED=NO # ** TEST SUCCEEDED ** 32 tests 0 failures First of three stacked PRs against #6: 1. (this PR) cancel + cleanup 2. size + SHA-256 validation 3. retry UX polish Refs #6 * feat: Validate model downloads against expected size and SHA-256 (#6) Adds integrity checks for curated model downloads. After a download finishes, the file is moved to a staging URL, validated against expected size + SHA-256, and only then swapped into the install location. If validation fails, the staged file is removed and the prior install (if any) stays in place — a corrupt download can't take out a working copy. Validation strategy: both size and SHA-256. - Size is a fast pre-check that catches truncated downloads. - SHA-256 is the integrity guarantee against silent corruption (bit flips, partial writes, mirror substitution). - Either alone is insufficient; together they're cheap. Catalog now carries `expectedSizeBytes` and `sha256` for the three curated models. Values were captured from HuggingFace's `x-linked-size` and `x-linked-etag` response headers — refresh procedure documented in the catalog. Both fields are Optional; validators no-op when nil so future catalog additions don't have to wait on metadata to land. Streams the SHA-256 in 1 MB chunks so the largest curated model (~4.5 GB) doesn't get loaded entirely into memory. Tests (11 new, 43 total): - validateSize: match, mismatch (both directions), nil expected, missing file - validateSHA256: known-good hash, uppercase tolerance, mismatch, nil expected, missing file - streaming guard: 3 MB fixture exercises the multi-chunk read loop so a regression to single-shot read would fail the test Verified locally: xcodebuild test -project tabby.xcodeproj -scheme tabby \ -destination 'platform=macOS' CODE_SIGNING_ALLOWED=NO # ** TEST SUCCEEDED ** 43 tests 0 failures Stacked on #60 (download cancel). Folds into #6. Refs #6 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: FuJacob <141651335+FuJacob@users.noreply.github.com>
Owner
|
@copilot resolve the merge conflicts in this pull request |
1 similar comment
Owner
|
@copilot resolve the merge conflicts in this pull request |
…ancel # Conflicts: # tabby.xcodeproj/project.pbxproj Co-authored-by: FuJacob <141651335+FuJacob@users.noreply.github.com>
Resolved in |
The branch is already up to date with |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First of three stacked PRs against #6. Adds a working cancel control for in-flight model downloads. Until now, the only way to stop a 3.5GB download in progress was to quit the app —
Task.cancel()flippedTask.isCancelledbut the URLSession download kept running.Validation
7 new tests in
DownloadOutcomeClassifierTests. URLSession-level cancellation propagation is exercised end-to-end on real downloads but not unit-tested directly (would require URLProtocol gymnastics for marginal additional coverage).UI verified by inspection — the cancel button renders next to the progress percent during
.downloadingand is a no-op tap target outside that state.Linked issues
Refs #6 (acceptance criteria 1 and 2: cancel + cleanup)
Risk / rollout notes
cancel(filename:)→Task.cancel()→withTaskCancellationHandler.onCancel→URLSessionDownloadTask.cancel()→didCompleteWithError(URLError.cancelled)→DownloadOutcomeClassifier.isUserCancellation→ restore.idle/.downloaded.DownloadOutcomeClassifier— pure helper, no side effects. The classification matters because Task.cancel() can surface as eitherCancellationError(cancelled before URLSession started) orURLError.cancelled(cancelled mid-flight). Without the classifier, the in-flight case routes through the catch-all and shows "operation couldn't be completed" instead of just rolling back.Open questions answered for the rest of #6