diff --git a/studio/backend/routes/settings.py b/studio/backend/routes/settings.py index 49e2f29d39..9639cee82d 100644 --- a/studio/backend/routes/settings.py +++ b/studio/backend/routes/settings.py @@ -355,32 +355,14 @@ 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 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 # 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 - # legacy flags on the first save, and auto-switch then prefers the qualified - # entry, so nothing could restore them. Requiring the suffix to be exactly - # the label the scanner derives for this filename is what keeps an arbitrary - # colon-containing POSIX path out: "/models/foo:bar.gguf" splits to a head - # that is not a .gguf at all. - # Split on both separators rather than os.path.basename: a "C:\..." key is - # written on Windows but may be read back by a backend that is not, and - # there a backslash is an ordinary filename character. - filename = head.replace("\\", "/").rsplit("/", 1)[-1] - if head.lower().endswith(".gguf") and tail == extract_quant_label(filename): - return head - return None + # two files at the same base quant distinct, and a .gguf with no recognized + # token is labelled by its stem, so both forms count. + split = split_quant_suffix(model_id) + return split[0] if split is not None else None @router.put("/openai-auto-switch/overrides", response_model = ModelOverridesResponse) diff --git a/studio/backend/tests/test_openai_auto_switch.py b/studio/backend/tests/test_openai_auto_switch.py index c5f454654d..80d076a865 100644 --- a/studio/backend/tests/test_openai_auto_switch.py +++ b/studio/backend/tests/test_openai_auto_switch.py @@ -4745,6 +4745,19 @@ def test_a_posix_path_variant_folds_while_the_path_does_not(monkeypatch): assert settings.get_model_override("/models/foo:Q4_K_M") == {} +def test_an_unknown_gguf_label_is_reachable_in_either_casing(monkeypatch): + # A .gguf with no recognizable quant token is labelled by its stem, and v2 + # storage lowercases that label while the scanner probes with the filename's + # own casing. Folding only recognized quant labels left the migrated entry + # unreachable for exactly the files that need the fallback. + _mock_override_store(monkeypatch) + settings.set_model_override("/models/CustomModel.gguf:custommodel", max_seq_length = 8192) + got = settings.get_model_override("/models/CustomModel.gguf:CustomModel") + assert got["max_seq_length"] == 8192 + # The path itself is still case-sensitive on POSIX. + assert settings.get_model_override("/models/custommodel.gguf:CustomModel") == {} + + 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. diff --git a/studio/backend/utils/openai_auto_switch_settings.py b/studio/backend/utils/openai_auto_switch_settings.py index 59827121df..3f39525f7e 100644 --- a/studio/backend/utils/openai_auto_switch_settings.py +++ b/studio/backend/utils/openai_auto_switch_settings.py @@ -518,15 +518,28 @@ def split_quant_suffix(value: str) -> Optional[tuple[str, str]]: splitting it would graft /models/foo's launch flags onto a different model. """ from core.inference.llama_cpp import _GGUF_KNOWN_QUANT_RE + from hub.utils.gguf import extract_quant_label 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: + if "/" in tail or "\\" in tail: return None - if _GGUF_KNOWN_QUANT_RE.fullmatch(_BPW_SUFFIX.sub("", tail)) is None: + if len(tail) <= _MAX_QUANT_SUFFIX_LEN and _GGUF_KNOWN_QUANT_RE.fullmatch( + _BPW_SUFFIX.sub("", tail) + ): + return head, tail + # A .gguf whose filename holds no recognizable quant token is still labelled + # by the scanner, which falls back to the stem, so keys like + # "/models/CustomModel.gguf:custommodel" exist. Storage lowercases that + # label while the scanner probes with the filename's own casing, so the + # comparison is case-insensitive. Requiring the suffix to be exactly that + # label is what keeps an ordinary colon out: "/models/foo:bar.gguf" splits + # to a head that is not a .gguf at all. + if not head.lower().endswith(".gguf"): return None - return head, tail + filename = head.replace("\\", "/").rsplit("/", 1)[-1] + return (head, tail) if tail.casefold() == extract_quant_label(filename).casefold() else None def _fold_posix_path_variant(value: str) -> str: diff --git a/studio/frontend/src/features/model-picker/api/model-overrides.ts b/studio/frontend/src/features/model-picker/api/model-overrides.ts index bf8ad1aa58..4c4081acf5 100644 --- a/studio/frontend/src/features/model-picker/api/model-overrides.ts +++ b/studio/frontend/src/features/model-picker/api/model-overrides.ts @@ -13,6 +13,10 @@ import { authFetch } from "@/features/auth"; import { readFastApiError } from "@/lib/format-fastapi-error"; +import { + normalizeGgufVariantIdentity, + normalizeModelIdentity, +} from "../model-config/model-identity"; import type { PerModelConfig } from "../model-config/per-model-config"; const OVERRIDES_URL = "/api/settings/openai-auto-switch/overrides"; @@ -136,7 +140,14 @@ export async function putModelOverride( ggufVariant: string | null | undefined, config: PerModelConfig | null, ): Promise { - const key = modelOverrideKey(modelId, ggufVariant); + // Keyed by the folded identity, not the literal spelling: the backfill sends + // a legacy casing while a UI save sends the normalized one, and the backend + // resolves both to the same row, so raw strings would open two queues for one + // model and let them race again. + const key = modelOverrideKey( + normalizeModelIdentity(modelId), + normalizeGgufVariantIdentity(ggufVariant), + ); // Chain on the settled tail: a failed write must not cancel the next one. const previous = writesByKey.get(key) ?? Promise.resolve(); const write = previous diff --git a/tests/studio/test_model_picker_contracts.py b/tests/studio/test_model_picker_contracts.py index a22ee1f5c3..1141b67003 100644 --- a/tests/studio/test_model_picker_contracts.py +++ b/tests/studio/test_model_picker_contracts.py @@ -775,7 +775,10 @@ def test_override_writes_are_ordered_per_model(): src = " ".join(_read("features/model-picker/api/model-overrides.ts").split()) assert "const writesByKey = new Map>();" in src # Keyed by the same override key the server stores under. - assert "const key = modelOverrideKey(modelId, ggufVariant);" in src + # Folded, not literal: the backfill uses a legacy casing while a UI save + # uses the normalized one, and the backend resolves both to one row, so + # raw strings would open two queues for one model and race again. + assert "const key = modelOverrideKey( normalizeModelIdentity(modelId), normalizeGgufVariantIdentity(ggufVariant), );" in src # Chained on the settled tail, so one failed write cannot cancel the next. assert "previous .catch(() => {}) .then(() => sendModelOverride(" in src # Only the last writer clears the slot, or a queue still building loses order.