Key write queues by identity, and reach unknown GGUF labels in either casing
Two holes in the previous two commits, both found by the same review round. The per-model write queue keyed on the literal spelling, so the backfill's legacy casing and a UI save's normalized one opened two queues for one model and raced exactly as before. It now keys on the folded identity, which is what the backend resolves by. A .gguf with no recognizable quant token is labelled by its filename stem, and v2 storage lowercases that label while the scanner probes with the filename's own casing. Folding only recognized quant labels therefore left the migrated entry unreachable for precisely the files that need the stem fallback. The suffix rule now also accepts a case-insensitive match against the label the scanner derives for that filename, which keeps an ordinary colon out because the head still has to be a .gguf. _bare_model_id drops onto the same shared rule rather than repeating half of it.
This commit is contained in:
parent
423532cbcb
commit
5a15f4d099
5 changed files with 49 additions and 27 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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<void> {
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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<string, Promise<void>>();" 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.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue