diff --git a/studio/backend/routes/settings.py b/studio/backend/routes/settings.py index 3e4409ae3b..139538a3bd 100644 --- a/studio/backend/routes/settings.py +++ b/studio/backend/routes/settings.py @@ -355,16 +355,17 @@ def get_openai_auto_switch_overrides( def _bare_model_id(model_id: str) -> Optional[str]: """``repo`` for a ``repo:QUANT`` key, or None when there is no quant suffix.""" - from core.inference.llama_cpp import _GGUF_KNOWN_QUANT_RE from hub.utils.gguf import extract_quant_label + from utils.openai_auto_switch_settings import split_quant_suffix head, sep, tail = model_id.rpartition(":") if not sep or not head or not tail: return None - if "/" not in tail and "\\" not in tail and len(tail) <= _MAX_VARIANT_SUFFIX_LEN: - # Must actually look like a quant, not just like a short path segment. - if _GGUF_KNOWN_QUANT_RE.fullmatch(tail) is not None: - return head + # Must actually look like a quant, not just like a short path segment. The + # label may carry a bits-per-weight modifier ("IQ4_XS-3.53bpw"), which keeps + # two files at the same base quant distinct, so that form counts too. + if split_quant_suffix(model_id) is not None: + return head # A .gguf with no recognizable quant token is still labelled by the scanner, # which falls back to the filename stem, so the UI stores keys like # "/models/custom.gguf:custom". Refusing those dropped the bare entry's diff --git a/studio/backend/tests/test_openai_auto_switch.py b/studio/backend/tests/test_openai_auto_switch.py index 721c9e2b22..cd77be0c77 100644 --- a/studio/backend/tests/test_openai_auto_switch.py +++ b/studio/backend/tests/test_openai_auto_switch.py @@ -4697,6 +4697,44 @@ def test_unknown_quant_label_on_a_gguf_still_carries_flags_over(monkeypatch): assert resp.overrides["/models/custom.gguf:custom"]["llama_extra_args"] == ["--flash-attn"] +def test_bpw_qualified_variants_still_carry_flags_over(monkeypatch): + # utils/models/model_config.py keeps a bits-per-weight modifier on the label + # so two files at the same base quant stay distinct, and that form reaches + # the override keys. The known-quant pattern does not accept it, so the bare + # entry was missed and the first qualified save dropped its launch flags. + import routes.settings as settings_route + + _mock_override_store(monkeypatch) + settings.set_model_override("unsloth/Repo-GGUF", llama_extra_args = ["--flash-attn"]) + resp = settings_route.update_openai_auto_switch_override( + settings_route.ModelOverridePayload( + model_id = "unsloth/Repo-GGUF:IQ4_XS-3.53bpw", max_seq_length = 4096 + ), + "tester", + ) + assert resp.overrides["unsloth/Repo-GGUF:IQ4_XS-3.53bpw"]["llama_extra_args"] == [ + "--flash-attn" + ] + + +def test_a_posix_path_variant_folds_while_the_path_does_not(monkeypatch): + # The browser lowercases the quant but keeps POSIX path casing, so the + # migrated key is "/models/Foo:q4_k_m" while the scanner asks for + # "/models/Foo:Q4_K_M". The path itself must still be case-sensitive. + _mock_override_store(monkeypatch) + settings.set_model_override("/models/Foo:q4_k_m", max_seq_length = 8192) + assert settings.get_model_override("/models/Foo:Q4_K_M")["max_seq_length"] == 8192 + assert settings.get_model_override("/models/foo:Q4_K_M") == {} + + +def test_a_posix_colon_filename_is_not_folded_as_a_variant(monkeypatch): + # "/models/foo:Bar.gguf" is one filename, not path + quant, so folding its + # tail would let it reach a different file's settings. + _mock_override_store(monkeypatch) + settings.set_model_override("/models/foo:bar.gguf", max_seq_length = 8192) + assert settings.get_model_override("/models/foo:Bar.gguf") == {} + + def test_a_suffix_the_scanner_would_not_derive_carries_nothing_over(monkeypatch): # Only the exact label the scanner derives for this filename is accepted, so # an unrelated colon suffix cannot reach into another model's flags. diff --git a/studio/backend/utils/openai_auto_switch_settings.py b/studio/backend/utils/openai_auto_switch_settings.py index 24fa7704f0..59827121df 100644 --- a/studio/backend/utils/openai_auto_switch_settings.py +++ b/studio/backend/utils/openai_auto_switch_settings.py @@ -502,6 +502,47 @@ def _fold_case_insensitive_path(model_id: str) -> Optional[str]: return trimmed.casefold() +# A quant label may carry a bits-per-weight modifier, because two files at the +# same base quant are kept distinct by it ("IQ4_XS-3.53bpw"). The two label +# helpers disagree on whether to keep it, so anything reading a stored key has +# to accept both forms. +_BPW_SUFFIX = re.compile(r"-[0-9]+(?:\.[0-9]+)?bpw$", re.IGNORECASE) +_MAX_QUANT_SUFFIX_LEN = 64 + + +def split_quant_suffix(value: str) -> Optional[tuple[str, str]]: + """``(head, quant)`` for a ``head:QUANT`` key, or None when there is none. + + The suffix has to be a real quant label, so an ordinary colon inside a POSIX + filename is left alone: "/models/foo:bar.gguf" is one valid filename, and + splitting it would graft /models/foo's launch flags onto a different model. + """ + from core.inference.llama_cpp import _GGUF_KNOWN_QUANT_RE + + head, sep, tail = value.rpartition(":") + if not sep or not head or not tail: + return None + if len(tail) > _MAX_QUANT_SUFFIX_LEN or "/" in tail or "\\" in tail: + return None + if _GGUF_KNOWN_QUANT_RE.fullmatch(_BPW_SUFFIX.sub("", tail)) is None: + return None + return head, tail + + +def _fold_posix_path_variant(value: str) -> str: + """A POSIX path id with only its quant suffix folded. + + The browser lowercases the variant but keeps the path casing, so a stored + "/models/Foo:q4_k_m" has to be reachable from "/models/Foo:Q4_K_M" without + also making "/models/Foo.gguf" reachable from "/models/foo.gguf". + """ + split = split_quant_suffix(value) + if split is None: + return value + head, quant = split + return f"{head}:{quant.casefold()}" + + def get_model_overrides() -> dict[str, dict]: """Per-model launch configs keyed by model id (see normalize_model_override).""" raw = _cached_setting(MODEL_OVERRIDES_SETTING_KEY, None) @@ -544,11 +585,22 @@ def resolve_model_override_key(model_id: str) -> Optional[str]: # every migrated Windows entry unreachable until the user saved it again. if _looks_like_filesystem_path(model_id): folded = _fold_case_insensitive_path(model_id) - if folded is None: - return None + if folded is not None: - def fold(key: str) -> Optional[str]: - return _fold_case_insensitive_path(key) + def fold(key: str) -> Optional[str]: + return _fold_case_insensitive_path(key) + else: + # POSIX: the path itself stays case-sensitive, but the browser + # lowercases the quant suffix while keeping the path casing, so a + # migrated "/models/Foo:q4_k_m" has to stay reachable from the + # scanner's "/models/Foo:Q4_K_M". + folded = _fold_posix_path_variant(model_id) + + def fold(key: str) -> Optional[str]: + # A path only ever folds onto another path. + if not _looks_like_filesystem_path(key): + return None + return None if _fold_case_insensitive_path(key) else _fold_posix_path_variant(key) else: folded = model_id.casefold() diff --git a/studio/frontend/src/features/model-picker/api/migrate-model-overrides.ts b/studio/frontend/src/features/model-picker/api/migrate-model-overrides.ts index b5f908a109..3cfa571db3 100644 --- a/studio/frontend/src/features/model-picker/api/migrate-model-overrides.ts +++ b/studio/frontend/src/features/model-picker/api/migrate-model-overrides.ts @@ -75,8 +75,13 @@ export async function backfillModelOverrides(): Promise { const local = listPerModelConfigs().filter( // A quant means it is a GGUF, which is the only thing API auto-switch // resolves. Backfilling a safetensors config would claim an API behaviour - // that does not exist. - (entry) => entry.ggufVariant != null && !isDefaultConfig(entry.config), + // that does not exist. A standalone .gguf picked directly carries no quant + // to select between, so it is stored with a null variant and would fail + // that test despite being exactly what auto-switch does resolve; the flag + // is then set and its settings stay browser-only for good. + (entry) => + (entry.ggufVariant != null || entry.modelId.toLowerCase().endsWith(".gguf")) && + !isDefaultConfig(entry.config), ); if (local.length === 0) { markRan(); diff --git a/tests/studio/test_model_picker_contracts.py b/tests/studio/test_model_picker_contracts.py index 0a66f4efe3..79efdbf61e 100644 --- a/tests/studio/test_model_picker_contracts.py +++ b/tests/studio/test_model_picker_contracts.py @@ -733,3 +733,16 @@ def test_api_reach_copy_is_limited_to_gguf_models(): src = " ".join(_read("features/hub/catalog/hub-model-settings-view.tsx").split()) assert "{target.isGguf ?" in src assert "Saved settings apply everywhere Studio loads this model." in src + + +def test_backfill_includes_a_standalone_gguf_with_no_variant(): + """A standalone .gguf picked directly has no quant to choose between, so it + is stored with a null variant. The quant filter classified it like + safetensors and skipped it, and since the done flag is set on the same pass + those settings stayed browser-only permanently while API auto-switch, which + does resolve that model, kept loading it with defaults. + """ + src = " ".join(_read("features/model-picker/api/migrate-model-overrides.ts").split()) + assert 'entry.modelId.toLowerCase().endsWith(".gguf")' in src + # Still excluded for safetensors, which auto-switch does not resolve. + assert "entry.ggufVariant != null ||" in src