-
-
Notifications
You must be signed in to change notification settings - Fork 671
fix(chrome-extension): resolve Hugging Face file URLs #1947
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,7 +30,38 @@ export function scanDocumentForDatasets() { | |
| } | ||
| }; | ||
|
|
||
| const huggingFaceHost = (url) => /^(?:huggingface\.co|hf\.co)$/i.test(url.hostname); | ||
|
|
||
| // The Hub links one file from seven routes -- blob, raw, blame, edit, delete, | ||
| // commits and the ?download=true button -- and every one of them ends in the | ||
| // file's own extension, so a repository page yields near-duplicate hits where | ||
| // only `resolve` (which 302s to the CDN) serves bytes a map source can read. | ||
| const huggingFaceFileUrl = (url) => { | ||
| if (!huggingFaceHost(url)) return null; | ||
| const parts = url.pathname.split("/").filter(Boolean); | ||
| const isRoute = (part) => /^(?:blob|raw|blame|edit|delete|commits|resolve)$/.test(part ?? ""); | ||
| // /<owner>/<repo>/<route>/<revision>/<path> for models, one segment deeper | ||
| // under /datasets and /spaces, and one shallower for the namespaceless | ||
| // legacy repos both shapes still carry. Read the route from the positions | ||
| // the grammar allows rather than scanning for the first keyword, so an | ||
| // owner, repository or revision named after a route cannot stand in for | ||
| // one, and prefer the deeper position since namespaced repos are the norm. | ||
| const route = (/^(?:datasets|spaces)$/.test(parts[0]) ? [3, 2] : [2, 1]).find( | ||
| (index) => isRoute(parts[index]) && parts.length >= index + 3, | ||
| ); | ||
| if (route === undefined) return null; | ||
| parts[route] = "resolve"; | ||
| const canonical = new URL(url.href); | ||
| canonical.pathname = `/${parts.join("/")}`; | ||
| canonical.searchParams.delete("download"); | ||
| // A line anchor off a blob page would otherwise split one file into two | ||
| // entries that the CDN serves identically. | ||
| canonical.hash = ""; | ||
| return canonical; | ||
| }; | ||
|
giswqs marked this conversation as resolved.
|
||
|
|
||
| const canonicalUrl = (url) => { | ||
| if (huggingFaceHost(url)) return huggingFaceFileUrl(url) ?? url; | ||
| if (url.hostname !== "source.coop") return url; | ||
| const parts = url.pathname.split("/").filter(Boolean); | ||
| if (parts.length < 3) return url; | ||
|
|
@@ -93,10 +124,18 @@ export function scanDocumentForDatasets() { | |
| return; | ||
| } | ||
|
|
||
| // Every Hub route other than a file route is a UI page -- tree, viewer, the | ||
| // "Auto-converted to Parquet" branch -- so a hint-based match there would | ||
| // offer HTML as data. | ||
| const onHub = huggingFaceHost(url); | ||
| if (onHub && !huggingFaceFileUrl(url)) return; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit (low confidence, minor perf): |
||
|
|
||
| const kind = classify(url, hint); | ||
| if (!kind) return; | ||
| const existing = datasets.get(url.href); | ||
| const name = label.trim() || cleanName(url); | ||
| // Hub links carry UI chrome as their text ("Download", "History", "308 kB | ||
| // xet"), so the file name has to come from the path. | ||
| const name = (onHub ? "" : label.trim()) || cleanName(url); | ||
| const candidate = { | ||
| url: url.href, | ||
| name, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug (medium confidence): the "prefer the deeper index" heuristic can misfire when a legacy (namespaceless) repo has a revision name that happens to collide with a route keyword and the file path is nested — both fairly plausible on the Hub.
Example:
https://huggingface.co/datasets/mnist/blob/raw/data/train.csv, wheremnistis a namespaceless dataset repo,blobis the real route, andrawis (coincidentally) the branch/revision name — not the download route.Walking the algorithm:
parts = ["datasets","mnist","blob","raw","data","train.csv"](length 6). The namespaced candidate is checked first:isRoute(parts[3])→isRoute("raw")→true, andparts.length >= 3+3→6 >= 6→true, so index 3 wins even though this repo has no namespace.parts[3]("raw") gets overwritten with"resolve", butparts[2]("blob", the actual route) is left untouched, producing/datasets/mnist/blob/resolve/data/train.csv— still ablob(HTML) URL, not a working direct-file link. The correct canonical URL is/datasets/mnist/resolve/raw/data/train.csv.The length check disambiguates most of the time (as covered by the existing tests), but it can't distinguish "namespaced repo, route at index 3" from "namespaceless repo with nested path, whose revision name happens to look like a route" — both satisfy
length >= 6. This is a narrow edge case (needs a revision literally namedblob/raw/blame/edit/delete/commits/resolve), but namespaceless repos (glue,squad,mnist,gpt2, …) and nested file paths are both common on the Hub, so it's not purely theoretical.