Skip to content

Commit 70d7d63

Browse files
committed
Address the typed models/ URI where the parser maps it
I rejected this the first time Codex raised it, and I was wrong. I checked REPO_TYPES_URL_PREFIXES on 0.36.2, saw no models/ entry, and concluded the form was unsupported. It is supported from 1.16, which routes resolve_path through parse_hf_uri: 1.25.1 resolve_path("models/openai-community/gpt2") -> model openai-community/gpt2 0.36.2 resolve_path("models/openai-community/gpt2") -> FileNotFoundError So the prefilter was calling an addressable model absent on current releases. That is the branch's own thesis turned on a shape I introduced, and it is the second time I have answered a question about resolve_path from the wrong version's constants. Gated on the parser, not on a version. REPO_TYPES_MAPPING contains models on both releases so it cannot discriminate, and stripping the prefix unconditionally would be worse than leaving it: on 0.36.2 that probes org/repo and answers True for an address that version cannot reach. _hub_addresses_typed_model_uris asks parse_hf_uri directly and answers False when it is absent. Deliberately narrow. parse_hf_uri reads models/a/b/c as repo a/b plus file c, and /abs/base as repo abs/base, so this file keeps its own path rules rather than delegating them: /models/org/repo, ./models/org/repo, ~/models/org/repo and models/a/b/c all stay False with the capability forced on. datasets/, spaces/ and kernels/ resolve too and stay rejected, since none can be the base model of a merge. Tests assert the classification equals the capability rather than hardcoding either answer, so they hold across the supported range. Behavioral gate 593 passed, 2 skipped.
1 parent ab6e780 commit 70d7d63

2 files changed

Lines changed: 87 additions & 0 deletions

File tree

tests/test_hub_transport_errors_single_segment.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,53 @@ def test_a_single_segment_id_keeps_its_revision(name):
257257
assert saving_utils._is_hub_repo_id(name) is True
258258

259259

260+
@pytest.mark.parametrize("name", [
261+
"models/openai-community/gpt2",
262+
"hf://models/openai-community/gpt2",
263+
"models/openai-community/gpt2@main",
264+
])
265+
def test_a_typed_model_uri_tracks_what_the_parser_does(name):
266+
"""`models/namespace/name` is the typed form of a model address.
267+
268+
Whether it resolves is a property of the installed release, not of a version
269+
number, so this asserts the classification *equals the capability* rather than
270+
hardcoding either answer:
271+
272+
1.25.1 resolve_path("models/openai-community/gpt2") -> model openai-community/gpt2
273+
0.36.2 resolve_path("models/openai-community/gpt2") -> FileNotFoundError
274+
275+
Stripping the prefix unconditionally would be worse than not stripping it, since
276+
on 0.36.2 that probes `openai-community/gpt2` and would answer True for an
277+
address that version cannot reach. Caught by Codex, twice: I rejected it the
278+
first time from 0.36.2's prefix table, which is the wrong version to check.
279+
"""
280+
supported = saving_utils._hub_addresses_typed_model_uris()
281+
assert saving_utils._is_hub_repo_id(name) is supported
282+
283+
284+
@pytest.mark.parametrize("name", [
285+
"/models/org/repo", "./models/org/repo", "~/models/org/repo", "models/a/b/c",
286+
])
287+
def test_the_typed_prefix_never_launders_a_path(monkeypatch, name):
288+
"""With the capability forced on, a path that merely begins with `models` stays a
289+
path. `parse_hf_uri` itself would read `models/a/b/c` as repo `a/b` plus file `c`,
290+
and `/abs/base` as repo `abs/base`, which is why this file does not delegate its
291+
path rules to that parser."""
292+
monkeypatch.setattr(saving_utils, "_hub_addresses_typed_model_uris", lambda: True)
293+
assert saving_utils._is_hub_repo_id(name) is False
294+
295+
296+
@pytest.mark.parametrize("name", [
297+
"datasets/stanfordnlp/imdb", "spaces/org/demo", "kernels/org/k",
298+
])
299+
def test_repo_types_that_cannot_be_a_base_model_stay_rejected(monkeypatch, name):
300+
"""These resolve too, and are rejected deliberately: a dataset, a Space and a
301+
kernel are not the base model of a LoRA merge, so a Hub probe for one is a
302+
round trip that can only answer a question nobody asked."""
303+
monkeypatch.setattr(saving_utils, "_hub_addresses_typed_model_uris", lambda: True)
304+
assert saving_utils._is_hub_repo_id(name) is False
305+
306+
260307
@pytest.mark.parametrize("save_method", ["merged_4bit", "forced_merged_4bit", "merged_16bit", None])
261308
def test_an_at_sign_in_a_local_directory_changes_nothing(monkeypatch, tmp_path, save_method):
262309
"""The argument the revision split rests on, which nothing else pins.

unsloth_zoo/saving_utils.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4516,6 +4516,40 @@ def find_lora_base_model(model_to_inspect):
45164516
# a message that does not say what the guard says was not raised by the guard.
45174517
_SINGLE_SEGMENT_REJECTION = re.compile(r"single[\s\-_]?segment|namespace/name", re.IGNORECASE)
45184518

4519+
def _hub_addresses_typed_model_uris():
4520+
"""Does the installed huggingface_hub read `models/namespace/name` as a model?
4521+
4522+
Asked of the parser `resolve_path` actually consults, never of a version number.
4523+
1.16+ routes through `parse_hf_uri`, which maps the optional `models/` type
4524+
prefix; 0.36.2 tests `REPO_TYPES_URL_PREFIXES.values()`, which holds only
4525+
`datasets/` and `spaces/`, so there `models/org/repo` resolves to nothing:
4526+
4527+
1.25.1 resolve_path("models/openai-community/gpt2") -> model openai-community/gpt2
4528+
0.36.2 resolve_path("models/openai-community/gpt2") -> FileNotFoundError
4529+
4530+
`REPO_TYPES_MAPPING` contains `models` on both, so it cannot tell them apart.
4531+
Stripping the prefix unconditionally would be worse than leaving it: on 0.36.2 it
4532+
would probe `org/repo` and answer True for an address that version cannot reach.
4533+
4534+
Only `models/` is worth asking about. `datasets/`, `spaces/` and `kernels/` all
4535+
resolve too, and are all rejected by the depth rule on purpose, because none of
4536+
them can be the base model of a LoRA merge.
4537+
"""
4538+
try:
4539+
from huggingface_hub.utils._hf_uris import parse_hf_uri
4540+
except Exception:
4541+
try:
4542+
from huggingface_hub.hf_file_system import parse_hf_uri
4543+
except Exception:
4544+
return False
4545+
try:
4546+
parsed = parse_hf_uri("hf://models/namespace/name")
4547+
except Exception:
4548+
return False
4549+
return getattr(parsed, "type", None) == "model" and getattr(parsed, "id", None) == "namespace/name"
4550+
pass
4551+
4552+
45194553
def _as_hub_addressed(model_name):
45204554
"""The repo id part of a name, the way `HfFileSystem` itself addresses it.
45214555
@@ -4538,6 +4572,12 @@ def _as_hub_addressed(model_name):
45384572
"""
45394573
name = str(model_name)
45404574
if name.startswith("hf://"): name = name[len("hf://"):]
4575+
# `models/namespace/name` is the typed form of the same model address, and where
4576+
# the installed version reads it that way its three segments must not read as a
4577+
# filesystem path. Gated on the parser rather than stripped outright, because on
4578+
# a version that does not map the prefix this would probe a different repo.
4579+
if name.startswith("models/") and _hub_addresses_typed_model_uris():
4580+
name = name[len("models/"):]
45414581
# Split a revision only when what precedes it is addressable, so an `@` inside
45424582
# an ordinary path is left alone. `gpt2@main` counts: the zero-slash branch of
45434583
# `resolve_path` splits `@` too, so a canonical single segment id carries a

0 commit comments

Comments
 (0)