mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-20 22:34:00 +00:00
Clear the four main CI reds blocking every open PR (#8506)
* Stop the pre-quant allowlist tests depending on the host's torchao Backend CI has been red on main since the pre-quant allowlist landed. 19 tests in studio/backend/tests/test_diffusion_prequant.py fail with assert None is not None and one with "this torch cannot register the pre-quant constructor allowlist". The cause is not the allowlist. _register_prequant_safe_globals refuses unless _prequant_safe_globals resolves torch.torch_version.TorchVersion plus at least one real torchao class, by actual import. The Backend tests job installs torch 2.10.0+cpu and transformers and no torchao at all, so nothing resolves, every load refuses, and every assertion about the loader lands on the refusal instead. The tests also swap sys.modules["torch"] for a bare module while a load runs, so even torch.torch_version is unimportable at that moment, torchao or no torchao. The file's own header says these tests run "without CUDA, torchao, or a real diffusers model", so this restores what was intended rather than changing it. An autouse fixture stands in for the entries the host could NOT import and leaves the rest alone, so a host that has torchao still tests the real classes. Standing in everywhere would have hidden the thing the stand-in is standing in for, so two tests keep the real gate under test: test_the_registration_floor_needs_a_real_torchao asks the unpatched resolver and skips where there is no torchao to ask. AffineQuantizedTensor is already deprecated upstream (pytorch/ao#2752), so a release that retires it should fail here rather than pass on a fabricated class. test_the_registration_refuses_when_nothing_resolves pins the CI image's own situation: an install that cannot express the allowlist gets a raise and a dense fallback, never a torch.load with the restriction dropped. Verified both ways. With torchao installed: 83 passed. With torchao made unimportable by a ModuleNotFoundError-raising meta_path finder, which reproduces the CI failure exactly on this branch's parent (19 failed): 82 passed, 1 skipped. * Expect the 404 a hub-style id now earns after a stale hit is refreshed away test_an_expired_positive_hit_refreshes_before_switching has failed on main since the mistyped-GGUF-id fix landed. It passes at that commit's parent and fails at the commit, in isolation as well as in a full run, so it is not a flake or an ordering effect. That fix made unsloth/ and -GGUF ids concrete references, which is the point of it: a wrong model name is refused instead of being answered by whatever happens to be loaded. This test's scenario is exactly that shape. The index holds a stale positive for unsloth/B-GGUF, the refresh proves it is gone, and the request names it anyway, so a 404 is now the correct outcome and the test's "the hook returns" expectation is the stale part. What the test is actually about is unchanged and still asserted: one rescan, then a re-resolve that does not rescan, no load attempted, and the resident model left alone. The refusal is asserted alongside those rather than the test being loosened to swallow it. The third resolve call now expected is the refusal wording asking what the resident model is; it passes allow_scan = False, which is why the rescan count is still one. Verified: tests/test_openai_auto_switch.py, tests/test_stt_sidecar.py and tests/test_diffusion_prequant.py together, 630 passed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Follow Normalize to base/modules, where ST master moved it The sentence-transformers pinned-symbol matrix is red on main: FAILED test_st_models_re_exports[master] master: ST 5.4+ layout: class Normalize not found in any of ['sentence_transformers/sentence_transformer/modules/normalize.py', 'sentence_transformers/sentence_transformer/Normalize.py'] Upstream moved it to sentence_transformers/base/modules/normalize.py, which is where the reorg has been heading: Transformer in the same table already lists its base/modules path first. Checked per ref rather than assumed. On master, base/modules/normalize.py has the class and sentence_transformer/modules/normalize.py is gone; on v5.6.0 and v5.4.1 it is the other way round. Both spellings therefore stay listed, and every tag in ST_TAGS resolves. Pooling is left alone. It has not moved, and listing a path that does not exist anywhere would weaken the check rather than future-proof it. Verified: 40 passed. * Follow four frontend contracts to where the code moved Repo tests (CPU) is red on main and on every open PR: 4 failed, 6084 passed. Reproduced locally on the merge base, same four. All four are source-string contracts that a refactor moved out from under, and in each case the behaviour they exist to protect is intact -- checked by reading the new code, not assumed. settleLostGeneration: the proof that a lost POST reached the backend was an inline knownIds set with !knownIds.has(image.id). It is now a baseline handed to hasUnknownRecord in lib/gallery-flags.ts. Same proof through a named helper, so the assertion follows it and also pins that the helper exists. The GGUF footprint dedupe: effectiveRecommended went from a single quant to a SET of them, so "is this the recommended one" is membership rather than equality. The contract is unchanged -- the recommended quant represents its own group when it has one -- and both halves are asserted in the new spelling. The stale negated half is dropped rather than reworded, because !effectiveRecommended.has(current.quant) already says it. The provider backfill: a literal Promise.allSettled(backfillTasks) became settleTasksIfCurrent(backfillTasks, isCurrent) in features/credentials/reconciliation.ts, which allSettles them AND drops the result when the auth session has moved on. The assertion follows the call and then checks the helper still allSettles, so "one failing task must not sink the rest" is still under test rather than taken on the function's name. test_chat_page_hydrates_connections_on_startup is renamed to test_connections_are_hydrated_on_startup. The sync moved out of chat-page.tsx into features/credentials/bootstrap.ts, which the ROOT route calls -- a wider guarantee, not a narrower one, since connections now hydrate on any entry into the app rather than only on the chat page. Asserting the old location would fail on a change that improved the thing being asserted. Both halves are pinned: the bootstrap wires the sync, and something actually runs the bootstrap. 189 passed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: danielhanchen <unslothshared@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This commit is contained in:
parent
f20d465c75
commit
a23951b726
5 changed files with 133 additions and 15 deletions
|
|
@ -28,6 +28,38 @@ from core.inference.diffusion_prequant import (
|
|||
)
|
||||
|
||||
|
||||
@pytest.fixture(autouse = True)
|
||||
def real_prequant_safe_globals(monkeypatch):
|
||||
"""Stand in for the allowlist entries this host cannot import, and hand back the real resolver.
|
||||
|
||||
The file header promises these tests run without torchao, and the Backend CI image keeps that
|
||||
promise literally: it installs torch and transformers and no torchao at all. Production then
|
||||
resolves not one entry of ``_PREQUANT_SAFE_GLOBALS``, the registration floor ("TorchVersion
|
||||
plus at least one real torchao class") is not met, every load refuses, and 19 tests in here
|
||||
fail on ``assert None is not None`` -- saying nothing whatever about the loader they are
|
||||
about. The tests also swap ``sys.modules["torch"]`` for a bare module while a load runs, so
|
||||
even ``torch.torch_version`` is unimportable at that moment, torchao or no torchao.
|
||||
|
||||
Standing in only for the names that did NOT resolve keeps a host that has torchao testing the
|
||||
real classes. Which names a given release actually ships is asked separately, by
|
||||
``test_the_registration_floor_needs_a_real_torchao``, which skips rather than pretends -- and
|
||||
``test_the_registration_refuses_when_nothing_resolves`` pins the refusal itself, so the gate
|
||||
that the CI image trips is still under test rather than merely worked around.
|
||||
"""
|
||||
resolver = pq._prequant_safe_globals
|
||||
resolved = {name: obj for obj, name in resolver()}
|
||||
pairs = [
|
||||
(resolved.get(f"{module}.{name}") or type(name, (), {}), f"{module}.{name}")
|
||||
for module, name in pq._PREQUANT_SAFE_GLOBALS
|
||||
]
|
||||
monkeypatch.setattr(pq, "_prequant_safe_globals", lambda: pairs)
|
||||
# Per test rather than per process: the memo is a module global, so one test's registration
|
||||
# would otherwise decide the answer for every test that ran after it.
|
||||
monkeypatch.setattr(pq, "_SAFE_GLOBALS_REGISTERED", None)
|
||||
monkeypatch.setattr(pq, "_RESOLVED_SAFE_GLOBALS", set())
|
||||
return resolver
|
||||
|
||||
|
||||
# ── resolve_prequant_source ──────────────────────────────────────────────────────
|
||||
def _fam(prequant_repos = (), prequant_variant_repos = ()):
|
||||
return DiffusionFamily(
|
||||
|
|
@ -592,7 +624,9 @@ def test_the_checkpoint_is_deserialized_under_an_allowlist(monkeypatch):
|
|||
assert seen["safe_globals"], "the allowlist has to be registered before the load"
|
||||
|
||||
|
||||
def test_the_allowlist_names_every_constructor_the_hosted_checkpoints_use(monkeypatch):
|
||||
def test_the_allowlist_names_every_constructor_the_hosted_checkpoints_use(
|
||||
monkeypatch, real_prequant_safe_globals
|
||||
):
|
||||
"""The exact set read out of the pickles Studio actually resolves.
|
||||
|
||||
Surveyed with ``pickletools`` (no unpickling) over every hosted prequant repo the family
|
||||
|
|
@ -628,7 +662,38 @@ def test_the_allowlist_names_every_constructor_the_hosted_checkpoints_use(monkey
|
|||
monkeypatch.setattr(
|
||||
pq, "_PREQUANT_SAFE_GLOBALS", (("torchao.nowhere", "Nope"), ("collections", "OrderedDict"))
|
||||
)
|
||||
assert [name for _obj, name in pq._prequant_safe_globals()] == ["collections.OrderedDict"]
|
||||
assert [name for _obj, name in real_prequant_safe_globals()] == ["collections.OrderedDict"]
|
||||
|
||||
|
||||
def test_the_registration_floor_needs_a_real_torchao(real_prequant_safe_globals):
|
||||
"""On a host that HAS torchao, the real resolution must clear the floor.
|
||||
|
||||
Every other test in this file stands in for the names the host cannot import, which is what
|
||||
lets them run on the torchao-free CI image. That stand-in would also hide a torchao release
|
||||
that renamed or retired the constructors out from under us -- ``AffineQuantizedTensor`` is
|
||||
already deprecated upstream (pytorch/ao#2752). So this one asks the unpatched resolver, and
|
||||
skips where there is nothing to ask."""
|
||||
pytest.importorskip("torchao")
|
||||
resolved = {name for _obj, name in real_prequant_safe_globals()}
|
||||
assert "torch.torch_version.TorchVersion" in resolved
|
||||
assert [name for name in resolved if name.startswith("torchao.")], (
|
||||
"no torchao constructor resolved, so no pre-quant checkpoint could be opened: "
|
||||
+ repr(sorted(resolved))
|
||||
)
|
||||
|
||||
|
||||
def test_the_registration_refuses_when_nothing_resolves(monkeypatch):
|
||||
"""And where there IS nothing to ask, the load refuses rather than unpickling unrestricted.
|
||||
|
||||
This is the CI image's own situation -- torch installed, torchao not -- so it is worth
|
||||
pinning directly: an install that cannot express the allowlist gets a raise and a dense
|
||||
fallback, never a ``torch.load`` with the restriction dropped."""
|
||||
monkeypatch.setattr(pq, "_prequant_safe_globals", list)
|
||||
monkeypatch.setattr(pq, "_SAFE_GLOBALS_REGISTERED", None)
|
||||
assert pq._register_prequant_safe_globals() is False
|
||||
assert pq.restricted_prequant_load_supported("fp8") is False
|
||||
with pytest.raises(RuntimeError, match = "allowlist"):
|
||||
pq._torch_load_prequant("/x.pt", map_location = "cpu")
|
||||
|
||||
|
||||
def test_a_malicious_checkpoint_is_refused_before_it_executes():
|
||||
|
|
|
|||
|
|
@ -290,9 +290,22 @@ def test_an_expired_positive_hit_refreshes_before_switching(monkeypatch):
|
|||
|
||||
monkeypatch.setattr(resolver, "resolve_local_gguf", _resolve)
|
||||
|
||||
_run_hook("unsloth/B-GGUF")
|
||||
# #8389 made a hub-style id a CONCRETE reference, so a name the refresh just proved is not
|
||||
# here is refused rather than quietly answered by whatever is resident. This test predates
|
||||
# that and used to assert the hook returned; what it is actually about -- one rescan, then a
|
||||
# re-resolve that does not rescan, and the resident model left alone -- is unchanged, so the
|
||||
# refusal is asserted alongside it instead of the test being weakened to swallow it.
|
||||
with pytest.raises(HTTPException) as excinfo:
|
||||
_run_hook("unsloth/B-GGUF")
|
||||
|
||||
assert calls == [("unsloth/B-GGUF", {}), ("unsloth/B-GGUF", {"allow_scan": False})]
|
||||
assert excinfo.value.status_code == 404
|
||||
# The third call is the refusal wording asking what the RESIDENT model is; like the second it
|
||||
# passes allow_scan = False, which is why the rescan count below is still one.
|
||||
assert calls == [
|
||||
("unsloth/B-GGUF", {}),
|
||||
("unsloth/B-GGUF", {"allow_scan": False}),
|
||||
("unsloth/A-GGUF:Q4_K_M", {"allow_scan": False}),
|
||||
]
|
||||
assert scans == [1]
|
||||
assert rec.calls == []
|
||||
assert backend.model_identifier == "unsloth/A-GGUF"
|
||||
|
|
|
|||
|
|
@ -1692,11 +1692,15 @@ def test_a_lost_generate_post_must_prove_it_reached_the_backend():
|
|||
src = _read("features/images/images-page.tsx")
|
||||
fn = src[src.index("async function settleLostGeneration") :]
|
||||
fn = fn[: fn.index("\n}\n")]
|
||||
assert "knownIds" in fn and "sawActive" in fn
|
||||
assert "!knownIds.has(image.id)" in fn
|
||||
assert "sawActive" in fn
|
||||
# The proof used to be an inline knownIds set; it is now a baseline handed
|
||||
# to hasUnknownRecord in lib/gallery-flags.ts. Same proof, named helper, so
|
||||
# the assertion follows it rather than pinning the old spelling.
|
||||
assert "hasUnknownRecord(" in fn
|
||||
assert "baseline" in fn
|
||||
assert "did not reach the server" in fn
|
||||
# And the caller snapshots the ids BEFORE the POST.
|
||||
assert "new Set(galleryCache.images.map((image) => image.id))" in src
|
||||
flags = _read("lib/gallery-flags.ts")
|
||||
assert "export async function hasUnknownRecord" in flags
|
||||
|
||||
|
||||
def test_parallel_slots_setting_wired_end_to_end():
|
||||
|
|
@ -3363,8 +3367,11 @@ def test_the_gguf_footprint_is_resolved_per_dependency_group_not_per_repo():
|
|||
assert "new Map<string, GgufVariantDetail>()" in group
|
||||
assert 'const key = variant.dependency_key ?? "";' in group
|
||||
# The recommended quant still wins, but only inside its own group.
|
||||
assert "variant.quant === effectiveRecommended" in group
|
||||
assert "current.quant !== effectiveRecommended" in group
|
||||
# effectiveRecommended went from a single quant to a SET of them, so the
|
||||
# representative test is membership rather than equality. The contract is
|
||||
# unchanged: the recommended quant represents its own group when it has one.
|
||||
assert "effectiveRecommended.has(variant.quant)" in group
|
||||
assert "!effectiveRecommended.has(current.quant)" in group
|
||||
|
||||
|
||||
def test_every_footprint_group_gets_its_own_resolve_call():
|
||||
|
|
|
|||
|
|
@ -11,6 +11,8 @@ FRONTEND = REPO / "studio/frontend/src"
|
|||
PROVIDERS_API = FRONTEND / "features/chat/api/providers-api.ts"
|
||||
SYNC_PROVIDERS = FRONTEND / "features/chat/sync-external-providers.ts"
|
||||
CHAT_PAGE = FRONTEND / "features/chat/chat-page.tsx"
|
||||
CREDENTIAL_BOOTSTRAP = FRONTEND / "features/credentials/bootstrap.ts"
|
||||
ROOT_ROUTE = FRONTEND / "app/routes/__root.tsx"
|
||||
PROVIDERS_DB = REPO / "studio/backend/storage/providers_db.py"
|
||||
PROVIDERS_MODELS = REPO / "studio/backend/models/providers.py"
|
||||
|
||||
|
|
@ -39,7 +41,17 @@ def test_frontend_sync_backfills_local_models_to_backend():
|
|||
source = SYNC_PROVIDERS.read_text(encoding = "utf-8")
|
||||
assert "updateProviderConfig" in source
|
||||
assert "needsModelBackfill" in source
|
||||
assert "Promise.allSettled(backfillTasks)" in source
|
||||
# The backfill tasks are awaited as a batch, and one failing must not sink
|
||||
# the rest. That used to be a literal Promise.allSettled here; it is now
|
||||
# settleTasksIfCurrent in features/credentials/reconciliation.ts, which
|
||||
# allSettles them AND drops the result when the auth session has moved on.
|
||||
# Same guarantee through a named helper, so the assertion follows it.
|
||||
assert "settleTasksIfCurrent(backfillTasks" in source
|
||||
helper = (FRONTEND / "features/credentials/reconciliation.ts").read_text(encoding = "utf-8")
|
||||
assert "export async function settleTasksIfCurrent" in helper
|
||||
assert (
|
||||
"allSettled" in helper
|
||||
), "the batch must still settle rather than reject on the first failure"
|
||||
|
||||
|
||||
def test_frontend_sync_preserves_local_provider_options():
|
||||
|
|
@ -49,10 +61,25 @@ def test_frontend_sync_preserves_local_provider_options():
|
|||
assert "openaiContainerTtlMinutes" in source
|
||||
|
||||
|
||||
def test_chat_page_hydrates_connections_on_startup():
|
||||
source = CHAT_PAGE.read_text(encoding = "utf-8")
|
||||
assert "syncExternalProvidersFromBackend" in source
|
||||
assert "await hydratePersistedSettings()" in source
|
||||
def test_connections_are_hydrated_on_startup():
|
||||
"""Renamed from test_chat_page_hydrates_connections_on_startup, because the
|
||||
chat page is no longer where it happens.
|
||||
|
||||
The provider sync moved out of chat-page.tsx into
|
||||
features/credentials/bootstrap.ts, which the ROOT route calls. That is a
|
||||
wider guarantee, not a narrower one: connections now hydrate on any entry
|
||||
into the app rather than only on the chat page. Asserting the old location
|
||||
would fail on a change that improved the thing being asserted, so the
|
||||
assertion follows the call to where it went and pins both halves -- the
|
||||
bootstrap wires the sync, and something actually runs the bootstrap."""
|
||||
bootstrap = CREDENTIAL_BOOTSTRAP.read_text(encoding = "utf-8")
|
||||
assert "syncExternalProvidersFromBackend" in bootstrap
|
||||
root = ROOT_ROUTE.read_text(encoding = "utf-8")
|
||||
assert (
|
||||
"bootstrapPersistedCredentials" in root
|
||||
), "nothing calls the credential bootstrap, so no page hydrates connections"
|
||||
# The chat page still hydrates its own persisted settings.
|
||||
assert "hydratePersistedSettings()" in CHAT_PAGE.read_text(encoding = "utf-8")
|
||||
|
||||
|
||||
def test_providers_api_sends_models_to_backend():
|
||||
|
|
|
|||
|
|
@ -75,6 +75,12 @@ def test_st_models_re_exports(tag: str):
|
|||
"sentence_transformers/sentence_transformer/Pooling.py",
|
||||
],
|
||||
"Normalize": [
|
||||
# ST master moved Normalize down beside Transformer under base/modules,
|
||||
# which is where the reorg has been heading: Transformer above already
|
||||
# lists its base/modules path first. Released 5.4 through 5.6 still keep
|
||||
# it under sentence_transformer/modules, so both spellings stay listed
|
||||
# and every tag in ST_TAGS resolves.
|
||||
"sentence_transformers/base/modules/normalize.py",
|
||||
"sentence_transformers/sentence_transformer/modules/normalize.py",
|
||||
"sentence_transformers/sentence_transformer/Normalize.py",
|
||||
],
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue