feat: Make download failure messages legible in the model catalog#72
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
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
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
Owner
|
@copilot resolve the merge conflicts in this pull request |
…etry-ux # Conflicts: # tabby.xcodeproj/project.pbxproj # tabby/Services/Utilities/ModelDownloadManager.swift Co-authored-by: FuJacob <141651335+FuJacob@users.noreply.github.com>
Resolved in
|
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
Third and last of the stacked PRs against #6. Lifts failure messages out of the metadata line into a dedicated wrapping row with a warning icon, and gives the Retry button a refresh symbol. Closes the last acceptance criterion: "failed downloads can be retried from the same UI."
The retry mechanism itself already worked (
Button("Retry") { onDownload() }); the gap was that validation errors from #61 are long enough to clip the size label or get truncated on smaller windows.Validation
No new tests — pure SwiftUI tweak, would require XCUITest scaffolding for marginal value.
Linked issues
Refs #6 (acceptance criterion 3: retry from same UI)
Risk / rollout notes
error.localizedDescriptionof whatever throws — same string the row showed before, just given more room.