-
-
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 2 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,31 @@ 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 | ||
| // for /datasets and /spaces (which still carry namespaceless legacy repos). | ||
| // Read the route from its fixed position rather than scanning, so an owner | ||
| // or repository named after a route cannot stand in for one. | ||
| const route = /^(?:datasets|spaces)$/.test(parts[0]) && isRoute(parts[3]) ? 3 : 2; | ||
| if (!isRoute(parts[route]) || parts.length < route + 3) return null; | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| parts[route] = "resolve"; | ||
|
Comment on lines
+43
to
+53
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. 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: Walking the algorithm: 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 |
||
| const canonical = new URL(url.href); | ||
| canonical.pathname = `/${parts.join("/")}`; | ||
| canonical.searchParams.delete("download"); | ||
| 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 +117,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, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.