Skip to content

Replace HubApi downloader with swift-huggingface HubClient - #315

Merged
mattt merged 17 commits into
mainfrom
mattt/swift-huggingface
Mar 3, 2026
Merged

Replace HubApi downloader with swift-huggingface HubClient#315
mattt merged 17 commits into
mainfrom
mattt/swift-huggingface

Conversation

@mattt

@mattt mattt commented Feb 19, 2026

Copy link
Copy Markdown
Collaborator

Alternative to #297

@mattt
mattt force-pushed the mattt/swift-huggingface branch from fde931a to 284ae43 Compare February 19, 2026 18:54
@mattt
mattt force-pushed the mattt/swift-huggingface branch from bcf5b81 to 5d91672 Compare February 23, 2026 18:04

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR replaces the existing HubApi download implementation with HubClient from the swift-huggingface package, updating download/progress behavior and adjusting tests accordingly.

Changes:

  • Add swift-huggingface as a dependency and import/use HubClient in HubApi.
  • Implement HubClient selection (cached/uncached, foreground/background) and add repo-id canonicalization for legacy model IDs.
  • Update download-related tests to reflect the new download/progress behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
Sources/Hub/HubApi.swift Switches file download path to HuggingFace.HubClient, adds progress bridging + repo ID resolution/cache, adjusts HEAD redirect handling by platform.
Tests/HubTests/HubApiTests.swift Updates assertions and progress expectation handling for the new download/progress behavior.
Package.swift Adds swift-huggingface dependency and links HuggingFace product into the Hub target.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Sources/Hub/HubApi.swift
Comment thread Tests/HubTests/HubApiTests.swift
Comment thread Tests/HubTests/HubApiTests.swift
Comment thread Sources/Hub/HubApi.swift
Comment thread Sources/Hub/HubApi.swift
Comment thread Sources/Hub/HubApi.swift

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Sources/Hub/HubApi.swift Outdated
Comment thread Package.swift Outdated
Comment thread Sources/Hub/HubApi.swift
Comment thread Sources/Hub/HubApi.swift
`destination` includes the filename as well, so we were creating
directories such as:

models
└── microsoft
    └── phi-4
        └── config.json
            └── config.json
Comment thread Sources/Hub/HubApi.swift Outdated
@pcuenca

pcuenca commented Feb 25, 2026

Copy link
Copy Markdown
Member

My download workaround fixes downloads for regular files but not for Xet ones.

I don't think we can solve this here. The problem, I think, is that copyFileToLocalDirectory should receive a directory destination (not the full file path), while downloadFileWithXet expects a file location.

So we either have to update copyFileToLocalDirectoryIfNeeded to remove the trailing filename (and revert my commit here), or append the filename to xetDestination.

@pcuenca

pcuenca commented Feb 25, 2026

Copy link
Copy Markdown
Member

The good news is that tests now proceed without deadlocks (after my second workaround) so we can see what's remaining.

Comment thread Sources/Hub/HubApi.swift Outdated
Comment thread Sources/Hub/HubApi.swift Outdated
@mattt

mattt commented Mar 2, 2026

Copy link
Copy Markdown
Collaborator Author

@pcuenca

My download workaround fixes downloads for regular files but not for Xet ones.

I don't think we can solve this here. The problem, I think, is that copyFileToLocalDirectory should receive a directory destination (not the full file path), while downloadFileWithXet expects a file location.

So we either have to update copyFileToLocalDirectoryIfNeeded to remove the trailing filename (and revert my commit here), or append the filename to xetDestination.

Whoops, sorry about that. I just opened a PR to fix upstream here: huggingface/swift-huggingface#43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Sources/Hub/HubApi.swift
Comment thread Sources/Hub/HubApi.swift
Comment thread Tests/HubTests/HubApiTests.swift
Comment thread Sources/Hub/HubApi.swift
Comment thread Package.swift Outdated

@pcuenca pcuenca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🔥

Co-authored-by: Pedro Cuenca <pedro@huggingface.co>
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.

3 participants