Read the concrete model key first, and fill settings in field by field
Three follow-ups on the last round. The auto-switch load tried the advertised id before the concrete load path, so an entry under the alias shadowed the settings the user had just saved and kept shadowing them. The settings page keys every local row by the path being loaded, while the alias is derived: the /v1/models name a hand-written overrides PUT is written against, and for a loose .gguf only its filename stem. Each pair now reads the path first, and the alias, the bare ids and the older filename-label key are all still read after it, so a cached repo (keyed by its repo id, which is the alias) resolves exactly as before. publicModelId collapses two paths that share a filename or a directory basename onto one id, so the resident check added last round could mark the wrong catalog row as loaded and seed its editor with another model's live launch config, then save that under this model's key. The Hub page now records the loadable identifier /status reports, as every other status reader already does, so the literal comparison names one row; the public-id pass only accepts a namespaced repo id, the one collapse that cannot name two models. The override map shipped before this browser mirror did, holding only llama_extra_args and max_seq_length, so an upgraded install can have a server entry for a model whose context, KV cache, speculative and GPU settings live only in localStorage. The backfill read key presence as done, skipped exactly those models and then marked itself complete, so their API loads lost the settings for good. Filling them in from the browser would reopen the race the conditional write just closed, so the merge is the server's: the PUT flag is now fill_absent_fields, and studio_db merges field by field under the write's own transaction, where a stored value always wins. A fill with an entry already there also stops replaying that entry's stored flags through validation, so one denylisted since it was saved cannot 400 the one-time migration. Tests: the key order and the fill in studio/backend/tests, the collapse in studio/frontend/tests, and the wiring in tests/studio/test_model_picker_contracts.py.
This commit is contained in:
danielhanchen
parent
d0e7b1914f
commit
82f637791d
12 changed files with 464 additions and 114 deletions
|
|
@ -4357,22 +4357,30 @@ async def _maybe_auto_switch_model(
|
|||
# Apply this model's saved launch config so an API swap loads
|
||||
# it exactly as the picker would. Try variant-qualified keys
|
||||
# first (two quants of one repo can differ), then bare ids, and
|
||||
# both the repo id and the load path (a local folder or a
|
||||
# non-active HF cache is configured against its path).
|
||||
# A standalone .gguf resolves with variant=None, but the picker
|
||||
# keys it by the quant label derived from the filename, so its
|
||||
# settings live under "<path>:LABEL" and no bare key reaches them.
|
||||
# within each pair the concrete load path before the advertised
|
||||
# id. The settings UI keys every local row (a folder, an LM
|
||||
# Studio dir, a non-active HF cache, a loose .gguf) by that path,
|
||||
# while override_id is a derived alias -- the /v1/models name a
|
||||
# hand-written overrides PUT uses, and for a loose file only its
|
||||
# filename stem. Reading the alias first let an older entry under
|
||||
# it shadow the settings the user just saved, for good. A cached
|
||||
# repo is keyed by its repo id, which is override_id, and no path
|
||||
# entry exists for it, so it still resolves on the second try.
|
||||
# A standalone .gguf resolves with variant=None; an early build
|
||||
# of this feature keyed it by the quant label derived from the
|
||||
# filename, so read "<path>:LABEL" too, after the bare path the
|
||||
# picker writes today.
|
||||
file_variant = None
|
||||
if not variant and target_id.lower().endswith(".gguf"):
|
||||
from hub.utils.gguf import extract_quant_label
|
||||
file_variant = extract_quant_label(os.path.basename(target_id))
|
||||
override = {}
|
||||
for override_key in (
|
||||
f"{override_id}:{variant}" if variant else None,
|
||||
f"{target_id}:{variant}" if variant else None,
|
||||
f"{override_id}:{variant}" if variant else None,
|
||||
target_id,
|
||||
f"{target_id}:{file_variant}" if file_variant else None,
|
||||
override_id,
|
||||
target_id,
|
||||
):
|
||||
if not override_key:
|
||||
continue
|
||||
|
|
|
|||
|
|
@ -182,11 +182,14 @@ class ModelOverridePayload(BaseModel):
|
|||
# Explicit intent: an all-default save carries no fields, which is shape
|
||||
# identical to "forget this model". None keeps the legacy contract.
|
||||
remove: Optional[bool] = None
|
||||
# Create, don't replace: the one-time localStorage backfill reads the map once
|
||||
# Fill in, don't replace: the one-time localStorage backfill reads the map once
|
||||
# and then writes each model in turn, so another tab saving during that pass
|
||||
# would be overwritten by this browser's older copy. The server tests and
|
||||
# writes under one transaction, which costs no extra round trip.
|
||||
only_if_absent: bool = False
|
||||
# would be overwritten by this browser's older copy. The server reads and writes
|
||||
# under one transaction, which costs no extra round trip. Field level, because an
|
||||
# install upgraded from a release that stored only llama_extra_args and
|
||||
# max_seq_length holds an entry the browser has the rest of, and skipping the
|
||||
# whole entry would strand exactly the settings this migration exists to carry.
|
||||
fill_absent_fields: bool = False
|
||||
|
||||
@field_validator("chat_template_override")
|
||||
@classmethod
|
||||
|
|
@ -376,17 +379,18 @@ def update_openai_auto_switch_override(
|
|||
from utils.openai_auto_switch_settings import get_model_override
|
||||
|
||||
try:
|
||||
if payload.only_if_absent and payload.remove is True:
|
||||
# A create that is also a delete has no meaning, and silently picking one
|
||||
if payload.fill_absent_fields and payload.remove is True:
|
||||
# A fill that is also a delete has no meaning, and silently picking one
|
||||
# would either lose settings or resurrect them.
|
||||
raise ValueError("only_if_absent cannot be combined with remove.")
|
||||
raise ValueError("fill_absent_fields cannot be combined with remove.")
|
||||
# Only model_id is the documented "remove". Otherwise omitted launch flags
|
||||
# carry over from the stored entry, since the settings UI cannot express them.
|
||||
requested_extra_args = payload.llama_extra_args
|
||||
# only_if_absent is a write mode, not a saved field: leaving it in would make
|
||||
# every payload look non-empty and break the legacy "no fields means remove".
|
||||
# fill_absent_fields is a write mode, not a saved field: leaving it in would
|
||||
# make every payload look non-empty and break the legacy "no fields means
|
||||
# remove".
|
||||
saved_fields = payload.model_dump(
|
||||
exclude = {"model_id", "llama_extra_args", "remove", "only_if_absent"},
|
||||
exclude = {"model_id", "llama_extra_args", "remove", "fill_absent_fields"},
|
||||
exclude_none = True,
|
||||
)
|
||||
if payload.remove is not None:
|
||||
|
|
@ -396,13 +400,19 @@ def update_openai_auto_switch_override(
|
|||
key: value for key, value in saved_fields.items() if key != "tensor_parallel"
|
||||
}
|
||||
if requested_extra_args is None and not is_removal:
|
||||
requested_extra_args = get_model_override(payload.model_id).get("llama_extra_args")
|
||||
if requested_extra_args is None:
|
||||
# First per-quant save for flags stored under the bare repo id.
|
||||
# Auto-switch prefers the qualified entry, so carry them over.
|
||||
bare_id = _bare_model_id(payload.model_id)
|
||||
if bare_id:
|
||||
requested_extra_args = get_model_override(bare_id).get("llama_extra_args")
|
||||
stored = get_model_override(payload.model_id)
|
||||
# A fill leaves every stored value alone, so an entry that is already
|
||||
# there keeps its flags without this echoing them back through
|
||||
# validation: one accepted when it was saved but denylisted since would
|
||||
# 400 the one-time migration, which then retries on every start.
|
||||
if not (payload.fill_absent_fields and stored):
|
||||
requested_extra_args = stored.get("llama_extra_args")
|
||||
if requested_extra_args is None:
|
||||
# First per-quant save for flags stored under the bare repo id.
|
||||
# Auto-switch prefers the qualified entry, so carry them over.
|
||||
bare_id = _bare_model_id(payload.model_id)
|
||||
if bare_id:
|
||||
requested_extra_args = get_model_override(bare_id).get("llama_extra_args")
|
||||
# Not validated on an explicit remove: nothing is stored, so a 400 would only
|
||||
# leave the override in place. A stale flag must not block forgetting.
|
||||
extra_args = [] if payload.remove is True else validate_extra_args(requested_extra_args)
|
||||
|
|
@ -431,7 +441,7 @@ def update_openai_auto_switch_override(
|
|||
gpu_layers = payload.gpu_layers,
|
||||
n_cpu_moe = payload.n_cpu_moe,
|
||||
gpu_ids = payload.gpu_ids,
|
||||
only_if_absent = payload.only_if_absent,
|
||||
fill_absent_fields = payload.fill_absent_fields,
|
||||
)
|
||||
except ValueError as exc:
|
||||
raise log_and_http_error(
|
||||
|
|
|
|||
|
|
@ -2963,17 +2963,20 @@ def upsert_app_setting_map_entry(
|
|||
entry_key: str,
|
||||
entry_value: dict[str, Any] | None,
|
||||
*,
|
||||
only_if_absent: bool = False,
|
||||
fill_absent_fields: bool = False,
|
||||
) -> dict[str, Any]:
|
||||
"""Set (or delete, when entry_value is falsy) one sub-entry of a dict-valued
|
||||
app setting, atomically under BEGIN IMMEDIATE so concurrent writers to other
|
||||
sub-entries cannot drop each other's updates.
|
||||
|
||||
``only_if_absent`` makes the write a create: an entry already there is left
|
||||
exactly as it is, and nothing is ever deleted. The test and the write share
|
||||
this transaction, so a caller that read the map earlier cannot replace a value
|
||||
written since. Used by the one-time localStorage backfill, whose contract is
|
||||
that the server copy is the newer authority.
|
||||
``fill_absent_fields`` writes only what is missing: the entry is created when
|
||||
it is not there, and otherwise gains the fields it does not already hold while
|
||||
every stored value is left exactly as it is. Nothing is ever deleted. The read
|
||||
and the write share this transaction, so a caller that read the map earlier
|
||||
cannot replace a value written since. Used by the one-time localStorage
|
||||
backfill, whose contract is that the server copy is the newer authority: an
|
||||
upgraded install can hold an entry with only the fields an older release knew,
|
||||
while this browser holds the rest, and entry-level skipping would strand them.
|
||||
"""
|
||||
conn = get_connection()
|
||||
try:
|
||||
|
|
@ -2982,11 +2985,20 @@ def upsert_app_setting_map_entry(
|
|||
current = _json_loads(row["value_json"], {}) if row else {}
|
||||
if not isinstance(current, dict):
|
||||
current = {}
|
||||
if only_if_absent:
|
||||
if not entry_value or entry_key in current:
|
||||
if fill_absent_fields:
|
||||
if not entry_value:
|
||||
conn.rollback()
|
||||
return current
|
||||
current[entry_key] = entry_value
|
||||
stored = current.get(entry_key)
|
||||
if isinstance(stored, dict):
|
||||
# Stored values win field by field, so this only ever adds.
|
||||
merged = {**entry_value, **stored}
|
||||
if merged == stored:
|
||||
conn.rollback()
|
||||
return current
|
||||
current[entry_key] = merged
|
||||
else:
|
||||
current[entry_key] = entry_value
|
||||
elif entry_value:
|
||||
current[entry_key] = entry_value
|
||||
else:
|
||||
|
|
|
|||
|
|
@ -706,14 +706,18 @@ def _mock_override_store(monkeypatch):
|
|||
entry_key,
|
||||
entry_value,
|
||||
*,
|
||||
only_if_absent = False,
|
||||
fill_absent_fields = False,
|
||||
):
|
||||
current = dict(store.get(key) or {})
|
||||
if only_if_absent:
|
||||
# Create only: an entry already there wins and nothing is deleted.
|
||||
if not entry_value or entry_key in current:
|
||||
if fill_absent_fields:
|
||||
# Fill only: every stored value wins and nothing is deleted.
|
||||
if not entry_value:
|
||||
return current
|
||||
current[entry_key] = entry_value
|
||||
stored = current.get(entry_key)
|
||||
if isinstance(stored, dict):
|
||||
current[entry_key] = {**entry_value, **stored}
|
||||
else:
|
||||
current[entry_key] = entry_value
|
||||
elif entry_value:
|
||||
current[entry_key] = entry_value
|
||||
else:
|
||||
|
|
@ -4343,9 +4347,11 @@ def test_override_found_under_a_concrete_path_with_variant(monkeypatch):
|
|||
assert rec.calls[0].max_seq_length == 8192
|
||||
|
||||
|
||||
def test_repo_qualified_override_beats_path_qualified(monkeypatch):
|
||||
# Most specific first, and the public repo id is what the picker configured
|
||||
# against.
|
||||
def test_path_qualified_override_beats_repo_qualified(monkeypatch):
|
||||
# Most specific first: the settings page keys a local row (a folder, an LM Studio
|
||||
# dir, a loose file) by the path being loaded, while the repo id is the advertised
|
||||
# alias that a second copy of the same repo, or a hand-written overrides PUT, is
|
||||
# configured under. The row the user actually edited wins.
|
||||
backend = _FakeBackend(None)
|
||||
rec = _LoadRecorder(backend)
|
||||
_wire(
|
||||
|
|
@ -4362,7 +4368,7 @@ def test_repo_qualified_override_beats_path_qualified(monkeypatch):
|
|||
monkeypatch.setattr(settings, "get_model_override", lambda mid: stored.get(mid, {}))
|
||||
|
||||
_run_hook("unsloth/B-GGUF")
|
||||
assert rec.calls[0].max_seq_length == 8192
|
||||
assert rec.calls[0].max_seq_length == 1024
|
||||
|
||||
|
||||
def test_first_quant_save_keeps_legacy_bare_repo_launch_flags(monkeypatch):
|
||||
|
|
@ -5192,11 +5198,11 @@ def test_abs_path_ids_are_recognised_in_either_platform_spelling():
|
|||
assert resolver._is_abs_path_id(repo_id) is False, repo_id
|
||||
|
||||
|
||||
def test_only_if_absent_put_never_replaces_a_newer_server_entry(monkeypatch):
|
||||
def test_fill_absent_fields_put_never_replaces_a_newer_server_value(monkeypatch):
|
||||
"""The one-time localStorage backfill reads the override map once and then
|
||||
writes each model in turn, so a save by another tab during that pass was
|
||||
overwritten by this browser's older copy. only_if_absent makes the write a
|
||||
create, so the entry already on the server wins."""
|
||||
overwritten by this browser's older copy. fill_absent_fields writes only what
|
||||
the entry lacks, so every value already on the server wins."""
|
||||
import routes.settings as settings_route
|
||||
|
||||
_mock_override_store(monkeypatch)
|
||||
|
|
@ -5207,22 +5213,72 @@ def test_only_if_absent_put_never_replaces_a_newer_server_entry(monkeypatch):
|
|||
|
||||
# The backfill's write, carrying this browser's older localStorage value.
|
||||
backfill = settings_route.ModelOverridePayload(
|
||||
model_id = "unsloth/B-GGUF", max_seq_length = 2048, only_if_absent = True
|
||||
model_id = "unsloth/B-GGUF", max_seq_length = 2048, fill_absent_fields = True
|
||||
)
|
||||
resp = settings_route.update_openai_auto_switch_override(backfill, "tester")
|
||||
assert resp.overrides["unsloth/B-GGUF"]["max_seq_length"] == 8192
|
||||
|
||||
# With nothing stored it still creates, or the migration would never run.
|
||||
fresh = settings_route.ModelOverridePayload(
|
||||
model_id = "unsloth/C-GGUF", max_seq_length = 2048, only_if_absent = True
|
||||
model_id = "unsloth/C-GGUF", max_seq_length = 2048, fill_absent_fields = True
|
||||
)
|
||||
resp2 = settings_route.update_openai_auto_switch_override(fresh, "tester")
|
||||
assert resp2.overrides["unsloth/C-GGUF"]["max_seq_length"] == 2048
|
||||
|
||||
|
||||
def test_only_if_absent_matches_a_legacy_casing_and_never_deletes(monkeypatch):
|
||||
def test_fill_absent_fields_carries_the_browser_only_settings_into_a_legacy_entry(monkeypatch):
|
||||
"""Codex P1: the override map shipped before the browser mirror did, storing only
|
||||
llama_extra_args and max_seq_length. An upgraded install holds such an entry while
|
||||
localStorage holds the context, KV cache, speculative and GPU settings, and an
|
||||
entry-level skip would strand exactly what the migration exists to carry."""
|
||||
import routes.settings as settings_route
|
||||
|
||||
store = _mock_override_store(monkeypatch)
|
||||
|
||||
legacy = settings_route.ModelOverridePayload(
|
||||
model_id = "unsloth/B-GGUF:Q4_K_M",
|
||||
llama_extra_args = ["--flash-attn"],
|
||||
max_seq_length = 8192,
|
||||
)
|
||||
settings_route.update_openai_auto_switch_override(legacy, "tester")
|
||||
|
||||
backfill = settings_route.ModelOverridePayload(
|
||||
model_id = "unsloth/B-GGUF:Q4_K_M",
|
||||
# The browser's own copy of a field the server already has, plus the ones
|
||||
# only it holds.
|
||||
max_seq_length = 2048,
|
||||
custom_context_length = 32768,
|
||||
kv_cache_dtype = "q8_0",
|
||||
speculative_type = "ngram",
|
||||
gpu_ids = [0, 1],
|
||||
fill_absent_fields = True,
|
||||
)
|
||||
resp = settings_route.update_openai_auto_switch_override(backfill, "tester")
|
||||
entry = resp.overrides["unsloth/B-GGUF:Q4_K_M"]
|
||||
# The server's own values survive untouched.
|
||||
assert entry["max_seq_length"] == 8192
|
||||
assert entry["llama_extra_args"] == ["--flash-attn"]
|
||||
# The browser-only settings are now there, so an API load applies them.
|
||||
assert entry["custom_context_length"] == 32768
|
||||
assert entry["kv_cache_dtype"] == "q8_0"
|
||||
assert entry["speculative_type"] == "ngram"
|
||||
assert entry["gpu_ids"] == [0, 1]
|
||||
# One entry, not two: the fill resolves onto the key a load reads.
|
||||
assert list(store[settings.MODEL_OVERRIDES_SETTING_KEY]) == ["unsloth/B-GGUF:Q4_K_M"]
|
||||
|
||||
# An ordinary save is still a replacement, or a settings edit could never
|
||||
# clear a field.
|
||||
edit = settings_route.ModelOverridePayload(
|
||||
model_id = "unsloth/B-GGUF:Q4_K_M", max_seq_length = 4096
|
||||
)
|
||||
resp2 = settings_route.update_openai_auto_switch_override(edit, "tester")
|
||||
assert resp2.overrides["unsloth/B-GGUF:Q4_K_M"]["max_seq_length"] == 4096
|
||||
assert "kv_cache_dtype" not in resp2.overrides["unsloth/B-GGUF:Q4_K_M"]
|
||||
|
||||
|
||||
def test_fill_absent_fields_matches_a_legacy_casing_and_never_deletes(monkeypatch):
|
||||
"""The stored key can carry the casing an older install typed, and it must not
|
||||
be duplicated or emptied by a create for the folded spelling."""
|
||||
be duplicated or emptied by a fill for the folded spelling."""
|
||||
import routes.settings as settings_route
|
||||
|
||||
_mock_override_store(monkeypatch)
|
||||
|
|
@ -5233,32 +5289,32 @@ def test_only_if_absent_matches_a_legacy_casing_and_never_deletes(monkeypatch):
|
|||
settings_route.update_openai_auto_switch_override(stored, "tester")
|
||||
|
||||
folded = settings_route.ModelOverridePayload(
|
||||
model_id = "unsloth/b-gguf:q4_k_m", max_seq_length = 2048, only_if_absent = True
|
||||
model_id = "unsloth/b-gguf:q4_k_m", max_seq_length = 2048, fill_absent_fields = True
|
||||
)
|
||||
resp = settings_route.update_openai_auto_switch_override(folded, "tester")
|
||||
assert list(resp.overrides) == ["Unsloth/B-GGUF:Q4_K_M"]
|
||||
assert resp.overrides["Unsloth/B-GGUF:Q4_K_M"]["max_seq_length"] == 8192
|
||||
|
||||
# An all-default create is a no-op, not the "empty payload means forget" path.
|
||||
# An all-default fill is a no-op, not the "empty payload means forget" path.
|
||||
empty = settings_route.ModelOverridePayload(
|
||||
model_id = "Unsloth/B-GGUF:Q4_K_M", only_if_absent = True
|
||||
model_id = "Unsloth/B-GGUF:Q4_K_M", fill_absent_fields = True
|
||||
)
|
||||
resp2 = settings_route.update_openai_auto_switch_override(empty, "tester")
|
||||
assert resp2.overrides["Unsloth/B-GGUF:Q4_K_M"]["max_seq_length"] == 8192
|
||||
|
||||
# A create that is also a delete has no meaning.
|
||||
# A fill that is also a delete has no meaning.
|
||||
with pytest.raises(HTTPException) as excinfo:
|
||||
settings_route.update_openai_auto_switch_override(
|
||||
settings_route.ModelOverridePayload(
|
||||
model_id = "Unsloth/B-GGUF:Q4_K_M", remove = True, only_if_absent = True
|
||||
model_id = "Unsloth/B-GGUF:Q4_K_M", remove = True, fill_absent_fields = True
|
||||
),
|
||||
"tester",
|
||||
)
|
||||
assert excinfo.value.status_code == 400
|
||||
|
||||
|
||||
def test_only_if_absent_does_not_break_the_empty_payload_removal(monkeypatch):
|
||||
"""only_if_absent is a write mode, not a saved field: leaving it in the dumped
|
||||
def test_fill_absent_fields_does_not_break_the_empty_payload_removal(monkeypatch):
|
||||
"""fill_absent_fields is a write mode, not a saved field: leaving it in the dumped
|
||||
payload would make every request look non-empty and silently retire the legacy
|
||||
"a payload carrying only model_id forgets this model" contract."""
|
||||
import routes.settings as settings_route
|
||||
|
|
@ -5272,9 +5328,9 @@ def test_only_if_absent_does_not_break_the_empty_payload_removal(monkeypatch):
|
|||
assert "unsloth/B-GGUF" not in resp.overrides
|
||||
|
||||
|
||||
def test_map_entry_create_tests_and_writes_in_one_transaction(tmp_path, monkeypatch):
|
||||
"""The real store, not the in-memory stand-in: the existence test has to share
|
||||
the write's transaction, or a concurrent writer still slips between them."""
|
||||
def test_map_entry_fill_reads_and_writes_in_one_transaction(tmp_path, monkeypatch):
|
||||
"""The real store, not the in-memory stand-in: the read has to share the write's
|
||||
transaction, or a concurrent writer still slips between them."""
|
||||
import storage.studio_db as db
|
||||
|
||||
monkeypatch.setenv("UNSLOTH_STUDIO_HOME", str(tmp_path))
|
||||
|
|
@ -5282,19 +5338,23 @@ def test_map_entry_create_tests_and_writes_in_one_transaction(tmp_path, monkeypa
|
|||
|
||||
key = "test_map_entry_create"
|
||||
assert db.upsert_app_setting_map_entry(key, "a", {"v": 1}) == {"a": {"v": 1}}
|
||||
# Present: left exactly as it is.
|
||||
assert db.upsert_app_setting_map_entry(key, "a", {"v": 2}, only_if_absent = True) == {
|
||||
# Present: the stored value stays, and a field it lacks is added.
|
||||
assert db.upsert_app_setting_map_entry(key, "a", {"v": 2}, fill_absent_fields = True) == {
|
||||
"a": {"v": 1}
|
||||
}
|
||||
assert db.get_app_setting(key) == {"a": {"v": 1}}
|
||||
assert db.upsert_app_setting_map_entry(
|
||||
key, "a", {"v": 2, "w": 7}, fill_absent_fields = True
|
||||
) == {"a": {"v": 1, "w": 7}}
|
||||
assert db.get_app_setting(key) == {"a": {"v": 1, "w": 7}}
|
||||
# Absent: created.
|
||||
assert db.upsert_app_setting_map_entry(key, "b", {"v": 3}, only_if_absent = True) == {
|
||||
"a": {"v": 1},
|
||||
assert db.upsert_app_setting_map_entry(key, "b", {"v": 3}, fill_absent_fields = True) == {
|
||||
"a": {"v": 1, "w": 7},
|
||||
"b": {"v": 3},
|
||||
}
|
||||
# A create never deletes, even with nothing to store.
|
||||
assert db.upsert_app_setting_map_entry(key, "a", None, only_if_absent = True) == {
|
||||
"a": {"v": 1},
|
||||
# A fill never deletes, even with nothing to store.
|
||||
assert db.upsert_app_setting_map_entry(key, "a", None, fill_absent_fields = True) == {
|
||||
"a": {"v": 1, "w": 7},
|
||||
"b": {"v": 3},
|
||||
}
|
||||
# The ordinary write still replaces and still removes.
|
||||
|
|
@ -5346,3 +5406,148 @@ def test_gpu_ids_payload_is_bounded():
|
|||
)
|
||||
# The ordinary case is untouched.
|
||||
assert settings_route.ModelOverridePayload(model_id = "x", gpu_ids = [0, 1]).gpu_ids == [0, 1]
|
||||
|
||||
|
||||
# ── codex round: the concrete key beats the advertised alias ──────────
|
||||
|
||||
|
||||
def _switch_with_overrides(monkeypatch, resolves_to, stored, requested):
|
||||
"""Run the auto-switch hook against a real override map and return the load."""
|
||||
backend = _FakeBackend(None)
|
||||
rec = _LoadRecorder(backend)
|
||||
_wire(monkeypatch, enabled = True, resolves_to = resolves_to, backend = backend, recorder = rec)
|
||||
_mock_override_store(monkeypatch)
|
||||
for key, max_seq_length in stored.items():
|
||||
settings.set_model_override(key, max_seq_length = max_seq_length)
|
||||
_run_hook(requested)
|
||||
assert len(rec.calls) == 1
|
||||
return rec.calls[0]
|
||||
|
||||
|
||||
def test_a_loose_gguf_prefers_its_path_keyed_settings_over_the_alias(monkeypatch):
|
||||
"""Codex: the settings UI keys a standalone .gguf by its bare path, while
|
||||
override_id is the filename stem /v1/models advertises and an overrides PUT can
|
||||
be written against. Reading the alias first let it shadow the saved settings for
|
||||
good, so an API load kept applying the old flags."""
|
||||
path = "/srv/models/Qwen3-8B-Q4_K_M.gguf"
|
||||
alias = "Qwen3-8B-Q4_K_M"
|
||||
req = _switch_with_overrides(
|
||||
monkeypatch,
|
||||
resolves_to = (path, None, alias),
|
||||
stored = {alias: 2048, path: 32768},
|
||||
requested = alias,
|
||||
)
|
||||
assert req.max_seq_length == 32768
|
||||
|
||||
# The alias is still read when it is the only key, so an entry written against
|
||||
# the advertised id keeps working.
|
||||
req2 = _switch_with_overrides(
|
||||
monkeypatch,
|
||||
resolves_to = (path, None, alias),
|
||||
stored = {alias: 2048},
|
||||
requested = alias,
|
||||
)
|
||||
assert req2.max_seq_length == 2048
|
||||
|
||||
|
||||
def test_the_filename_label_key_no_longer_shadows_the_bare_path(monkeypatch):
|
||||
"""An early build of this feature keyed a standalone .gguf by the quant label
|
||||
derived from its filename. Those entries stay readable, but the bare path the
|
||||
picker writes today comes first."""
|
||||
path = "/srv/models/Qwen3-8B-Q4_K_M.gguf"
|
||||
alias = "Qwen3-8B-Q4_K_M"
|
||||
req = _switch_with_overrides(
|
||||
monkeypatch,
|
||||
resolves_to = (path, None, alias),
|
||||
stored = {f"{path}:Q4_K_M": 2048, path: 32768},
|
||||
requested = alias,
|
||||
)
|
||||
assert req.max_seq_length == 32768
|
||||
|
||||
req2 = _switch_with_overrides(
|
||||
monkeypatch,
|
||||
resolves_to = (path, None, alias),
|
||||
stored = {f"{path}:Q4_K_M": 2048},
|
||||
requested = alias,
|
||||
)
|
||||
assert req2.max_seq_length == 2048
|
||||
|
||||
|
||||
def test_a_variant_qualified_path_key_beats_the_same_quant_under_the_alias(monkeypatch):
|
||||
"""An LM Studio dir or a non-active HF cache is configured against its path, so
|
||||
a same-quant entry under the repo id (another copy of the same repo, or a
|
||||
hand-written PUT) must not win over the row the user actually edited."""
|
||||
path = "/srv/lmstudio/publisher/Qwen3-8B-GGUF"
|
||||
repo = "publisher/Qwen3-8B-GGUF"
|
||||
req = _switch_with_overrides(
|
||||
monkeypatch,
|
||||
resolves_to = (path, "Q4_K_M", repo),
|
||||
stored = {f"{repo}:Q4_K_M": 2048, f"{path}:Q4_K_M": 32768},
|
||||
requested = f"{repo}:Q4_K_M",
|
||||
)
|
||||
assert req.max_seq_length == 32768
|
||||
|
||||
|
||||
def test_a_cached_repo_still_resolves_by_its_repo_id(monkeypatch):
|
||||
"""The Hub keys a cached repo row by its repo id, which is the advertised id,
|
||||
and no path entry exists for it, so it still resolves on the second try."""
|
||||
snapshot = "/mnt/old-cache/models--unsloth--Qwen3-8B-GGUF/snapshots/abc123"
|
||||
repo = "unsloth/Qwen3-8B-GGUF"
|
||||
req = _switch_with_overrides(
|
||||
monkeypatch,
|
||||
resolves_to = (snapshot, "Q4_K_M", repo),
|
||||
stored = {f"{repo}:Q4_K_M": 32768},
|
||||
requested = f"{repo}:Q4_K_M",
|
||||
)
|
||||
assert req.max_seq_length == 32768
|
||||
|
||||
# A bare entry under the repo id keeps working too.
|
||||
req2 = _switch_with_overrides(
|
||||
monkeypatch,
|
||||
resolves_to = (snapshot, "Q4_K_M", repo),
|
||||
stored = {repo: 16384},
|
||||
requested = f"{repo}:Q4_K_M",
|
||||
)
|
||||
assert req2.max_seq_length == 16384
|
||||
|
||||
|
||||
def test_a_fill_does_not_replay_a_stored_flag_through_validation(monkeypatch):
|
||||
"""The migration now writes for entries it used to skip, and an omitted
|
||||
llama_extra_args is normally carried over from the stored entry. Replaying a
|
||||
flag that has been denylisted since it was saved would 400 the one-time
|
||||
migration, which then retries on every start. A fill keeps the stored flags
|
||||
without sending them back."""
|
||||
import routes.settings as settings_route
|
||||
from core.inference import llama_server_args
|
||||
|
||||
store = _mock_override_store(monkeypatch)
|
||||
settings.set_model_override("unsloth/B-GGUF:Q4_K_M", llama_extra_args = ["--flash-attn"])
|
||||
|
||||
# The flag is refused from now on, as a later release's denylist would.
|
||||
real_validate = llama_server_args.validate_extra_args
|
||||
|
||||
def _reject_flash_attn(args):
|
||||
if args and "--flash-attn" in args:
|
||||
raise ValueError("--flash-attn is managed by the server.")
|
||||
return real_validate(args)
|
||||
|
||||
monkeypatch.setattr(llama_server_args, "validate_extra_args", _reject_flash_attn)
|
||||
|
||||
fill = settings_route.ModelOverridePayload(
|
||||
model_id = "unsloth/B-GGUF:Q4_K_M", custom_context_length = 32768, fill_absent_fields = True
|
||||
)
|
||||
resp = settings_route.update_openai_auto_switch_override(fill, "tester")
|
||||
entry = resp.overrides["unsloth/B-GGUF:Q4_K_M"]
|
||||
assert entry["llama_extra_args"] == ["--flash-attn"]
|
||||
assert entry["custom_context_length"] == 32768
|
||||
assert list(store[settings.MODEL_OVERRIDES_SETTING_KEY]) == ["unsloth/B-GGUF:Q4_K_M"]
|
||||
|
||||
# An ordinary save still validates what it is handed.
|
||||
with pytest.raises(HTTPException) as excinfo:
|
||||
settings_route.update_openai_auto_switch_override(
|
||||
settings_route.ModelOverridePayload(
|
||||
model_id = "unsloth/C-GGUF", llama_extra_args = ["--flash-attn"]
|
||||
),
|
||||
"tester",
|
||||
)
|
||||
assert excinfo.value.status_code == 400
|
||||
|
|
|
|||
|
|
@ -621,7 +621,7 @@ def set_model_override(
|
|||
llama_extra_args: Optional[list[str]] = None,
|
||||
max_seq_length: Optional[int] = None,
|
||||
*,
|
||||
only_if_absent: bool = False,
|
||||
fill_absent_fields: bool = False,
|
||||
**config: Any,
|
||||
) -> dict:
|
||||
"""Upsert one model's launch config; a config with no usable fields removes it.
|
||||
|
|
@ -629,9 +629,9 @@ def set_model_override(
|
|||
The two legacy parameters stay positional for existing callers; every other
|
||||
per-model field is passed by keyword and normalized together.
|
||||
|
||||
``only_if_absent`` turns the upsert into a create, leaving an entry already
|
||||
stored untouched. Returns the normalized entry either way; read the map back
|
||||
to see what is actually stored.
|
||||
``fill_absent_fields`` writes only what is missing: an entry already stored
|
||||
keeps every field it holds and gains only the ones it lacks. Returns the
|
||||
normalized entry either way; read the map back to see what is actually stored.
|
||||
"""
|
||||
if not model_id or not model_id.strip():
|
||||
raise ValueError("model_id is required.")
|
||||
|
|
@ -650,7 +650,7 @@ def set_model_override(
|
|||
MODEL_OVERRIDES_SETTING_KEY,
|
||||
model_id.strip(),
|
||||
entry or None,
|
||||
only_if_absent = only_if_absent,
|
||||
fill_absent_fields = fill_absent_fields,
|
||||
)
|
||||
_invalidate(MODEL_OVERRIDES_SETTING_KEY)
|
||||
return entry
|
||||
|
|
|
|||
|
|
@ -35,6 +35,7 @@ export type {
|
|||
GgufVariantDetail,
|
||||
InferenceStatusResponse,
|
||||
} from "./types/api";
|
||||
export { resolveInferenceCheckpointId } from "./lib/apply-inference-status-to-store";
|
||||
export {
|
||||
ChatSettingsPanel,
|
||||
ParamSlider,
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import {
|
|||
getInferenceStatus,
|
||||
isExternalModelId,
|
||||
listGgufVariants,
|
||||
resolveInferenceCheckpointId,
|
||||
useChatModelRuntime,
|
||||
useChatRuntimeStore,
|
||||
} from "@/features/chat";
|
||||
|
|
@ -384,16 +385,23 @@ export function ModelsPage() {
|
|||
void getInferenceStatus()
|
||||
.then((status) => {
|
||||
if (cancelled || !status.active_model) return;
|
||||
// The loadable identifier, as every other status reader records it: a GGUF
|
||||
// from a non-active HF cache or straight off disk loads by path, while
|
||||
// active_model is the clean public id (an HF snapshot's repo id, any other
|
||||
// file's filename stem). Two files that share a stem collapse onto one id,
|
||||
// so storing that would make the catalog row for one of them look loaded.
|
||||
const checkpointId = resolveInferenceCheckpointId(status);
|
||||
if (!checkpointId) return;
|
||||
const store = useChatRuntimeStore.getState();
|
||||
if (
|
||||
!isExternalModelId(store.params.checkpoint) &&
|
||||
(!modelIdsMatch(store.params.checkpoint, status.active_model) ||
|
||||
(!modelIdsMatch(store.params.checkpoint, checkpointId) ||
|
||||
!ggufVariantsMatch(
|
||||
store.activeGgufVariant,
|
||||
status.gguf_variant ?? null,
|
||||
))
|
||||
) {
|
||||
store.setCheckpoint(status.active_model, status.gguf_variant ?? null);
|
||||
store.setCheckpoint(checkpointId, status.gguf_variant ?? null);
|
||||
}
|
||||
})
|
||||
.catch(() => undefined);
|
||||
|
|
|
|||
|
|
@ -122,11 +122,19 @@ export function publicModelId(identifier: string): string {
|
|||
/**
|
||||
* Whether the model the backend reports as loaded is one of *candidates*.
|
||||
*
|
||||
* A GGUF loaded from an inactive HF cache or straight off disk is loaded by path,
|
||||
* but `/status` reports the clean public id, so an exact comparison against the
|
||||
* catalog row's path says "not loaded" and the caller falls back to saved or
|
||||
* default values instead of the live launch config. Candidates are compared
|
||||
* literally first, then by the public id the backend would report for them.
|
||||
* A GGUF loaded from an inactive HF cache is loaded by path, but a caller holding
|
||||
* only the public id would read an exact comparison against the catalog row's path
|
||||
* as "not loaded" and fall back to saved or default values instead of the live
|
||||
* launch config. Candidates are compared literally first, then by the public id.
|
||||
*
|
||||
* That second pass only accepts an identity that can name one model: an HF cache
|
||||
* snapshot collapses onto its repo id, which is globally unique, while every other
|
||||
* path collapses onto a filename or directory stem that two models can share
|
||||
* (`/models/alpha/model.gguf` and `/models/beta/model.gguf` are both "model").
|
||||
* Accepting a stem would mark the wrong row resident, seeding its editor with
|
||||
* another model's live config and saving it under this model's key. Callers with
|
||||
* the loadable identifier (`/status`'s `model_identifier`) pass it as the active
|
||||
* id, and the literal pass answers exactly.
|
||||
*/
|
||||
export function residentModelIdMatches(
|
||||
activeModelId: string | null | undefined,
|
||||
|
|
@ -142,7 +150,12 @@ export function residentModelIdMatches(
|
|||
}
|
||||
return candidates.some((candidate) => {
|
||||
const trimmed = candidate?.trim();
|
||||
return trimmed ? modelIdsMatch(active, publicModelId(trimmed)) : false;
|
||||
if (!trimmed) {
|
||||
return false;
|
||||
}
|
||||
const publicId = publicModelId(trimmed);
|
||||
// Unambiguous only when the collapse produced a namespaced repo id.
|
||||
return publicId.includes("/") && modelIdsMatch(active, publicId);
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -17,10 +17,12 @@ import {
|
|||
isDefaultConfig,
|
||||
listPerModelConfigs,
|
||||
} from "../model-config/per-model-config";
|
||||
import type { ApiModelOverride } from "./model-overrides";
|
||||
import {
|
||||
fetchModelOverrides,
|
||||
modelOverrideKey,
|
||||
putModelOverride,
|
||||
toApiOverride,
|
||||
} from "./model-overrides";
|
||||
|
||||
const DONE_FLAG = "unsloth_model_overrides_backfilled_v1";
|
||||
|
|
@ -62,9 +64,32 @@ function normalizedOverrideKey(key: string): string {
|
|||
}
|
||||
|
||||
/**
|
||||
* Push local configs the server has never seen. Never deletes and never overwrites:
|
||||
* an entry already there is the newer authority, and losing a setting would be
|
||||
* worse than leaving one unmigrated.
|
||||
* The fields *config* would contribute that the stored entry does not hold.
|
||||
*
|
||||
* A malformed entry (nothing constrains what an older install wrote into
|
||||
* app_settings) counts as holding nothing, so the migration still runs.
|
||||
*/
|
||||
function absentFields(
|
||||
stored: ApiModelOverride,
|
||||
config: Parameters<typeof toApiOverride>[0],
|
||||
): string[] {
|
||||
const fields = Object.keys(toApiOverride(config));
|
||||
if (typeof stored !== "object" || stored === null) {
|
||||
return fields;
|
||||
}
|
||||
return fields.filter((field) => !(field in stored));
|
||||
}
|
||||
|
||||
/**
|
||||
* Push local settings the server does not hold. Never deletes and never overwrites:
|
||||
* a value already there is the newer authority, and losing a setting would be worse
|
||||
* than leaving one unmigrated.
|
||||
*
|
||||
* Field by field, not entry by entry. The override map shipped before this browser
|
||||
* mirror did, storing only llama_extra_args and max_seq_length, so an upgraded
|
||||
* install can hold an entry for a model whose context, KV cache, speculative and GPU
|
||||
* settings live only here. Treating the key as done would skip exactly the settings
|
||||
* this migration exists to carry and then mark it complete.
|
||||
*/
|
||||
export async function backfillModelOverrides(): Promise<void> {
|
||||
if (alreadyRan()) {
|
||||
|
|
@ -97,7 +122,10 @@ export async function backfillModelOverrides(): Promise<void> {
|
|||
return;
|
||||
}
|
||||
|
||||
const known = new Set(Object.keys(existing).map(normalizedOverrideKey));
|
||||
const known = new Map<string, ApiModelOverride>();
|
||||
for (const [storedKey, storedEntry] of Object.entries(existing)) {
|
||||
known.set(normalizedOverrideKey(storedKey), storedEntry);
|
||||
}
|
||||
|
||||
let failed = false;
|
||||
for (const entry of local) {
|
||||
|
|
@ -106,9 +134,6 @@ export async function backfillModelOverrides(): Promise<void> {
|
|||
const key = normalizedOverrideKey(
|
||||
modelOverrideKey(entry.modelId, entry.ggufVariant),
|
||||
);
|
||||
if (known.has(key)) {
|
||||
continue;
|
||||
}
|
||||
// Re-read rather than trusting the snapshot from before the fetch: this write
|
||||
// is queued behind the interactive one and commits last, so a save or forget
|
||||
// during the round trip would be undone by it.
|
||||
|
|
@ -121,15 +146,20 @@ export async function backfillModelOverrides(): Promise<void> {
|
|||
if (!current || isDefaultConfig(current.config)) {
|
||||
continue;
|
||||
}
|
||||
const stored = known.get(key);
|
||||
// Nothing this browser could add, so skip the round trip entirely.
|
||||
if (stored && absentFields(stored, current.config).length === 0) {
|
||||
continue;
|
||||
}
|
||||
try {
|
||||
// Create only. `known` is a snapshot from before this loop started, so a save
|
||||
// by another tab during the pass is invisible here; the server does the test
|
||||
// and the write together rather than this re-fetching once per model.
|
||||
// Fills the gaps only. `known` is a snapshot from before this loop started, so
|
||||
// a save by another tab during the pass is invisible here; the server reads and
|
||||
// writes together rather than this re-fetching once per model.
|
||||
await putModelOverride(
|
||||
current.modelId,
|
||||
current.ggufVariant,
|
||||
current.config,
|
||||
{ onlyIfAbsent: true },
|
||||
{ fillAbsentFields: true },
|
||||
);
|
||||
} catch {
|
||||
failed = true;
|
||||
|
|
|
|||
|
|
@ -78,8 +78,11 @@ export async function fetchModelOverrides(): Promise<ApiModelOverrides> {
|
|||
* Only fields the user set are sent: the backend reads an absent field as "app
|
||||
* default", so nulls would pin defaults and stop the model following later global
|
||||
* changes. A `null` config means "no saved settings", which clears the entry.
|
||||
*
|
||||
* Exported so the one-time backfill can ask what this config would contribute and
|
||||
* compare it against the entry already on the server, field by field.
|
||||
*/
|
||||
function toApiOverride(config: PerModelConfig | null): ApiModelOverride {
|
||||
export function toApiOverride(config: PerModelConfig | null): ApiModelOverride {
|
||||
if (!config) {
|
||||
return {};
|
||||
}
|
||||
|
|
@ -130,14 +133,16 @@ const writesByKey = new Map<string, Promise<void>>();
|
|||
|
||||
export interface PutModelOverrideOptions {
|
||||
/**
|
||||
* Create only: leave an entry already on the server exactly as it is.
|
||||
* Fill in only what is missing: every value already on the server stays as it is.
|
||||
*
|
||||
* The one-time backfill reads the map once and then writes each model in turn,
|
||||
* so another tab saving during that pass would be overwritten by this browser's
|
||||
* older localStorage copy. The server tests and writes under one transaction,
|
||||
* which closes the window without a round trip per model.
|
||||
* The one-time backfill reads the map once and then writes each model in turn, so
|
||||
* another tab saving during that pass would be overwritten by this browser's older
|
||||
* localStorage copy. The server reads and writes under one transaction, which
|
||||
* closes the window without a round trip per model. Field level, so an entry an
|
||||
* older release stored with only its two fields still gains the browser-only ones
|
||||
* without the server losing anything it holds.
|
||||
*/
|
||||
onlyIfAbsent?: boolean;
|
||||
fillAbsentFields?: boolean;
|
||||
}
|
||||
|
||||
export async function putModelOverride(
|
||||
|
|
@ -183,9 +188,9 @@ async function sendModelOverride(
|
|||
model_id: modelOverrideKey(modelId, ggufVariant),
|
||||
// Only sent when set, so an older backend that does not know the field is
|
||||
// not handed an unexpected key by every ordinary save.
|
||||
...(options?.onlyIfAbsent
|
||||
...(options?.fillAbsentFields
|
||||
? // biome-ignore lint/style/useNamingConvention: API schema
|
||||
{ only_if_absent: true }
|
||||
{ fill_absent_fields: true }
|
||||
: {}),
|
||||
// Say which operation this is: an all-default save carries no fields, which is
|
||||
// shape-identical to "forget this model", and guessing wrong wipes launch flags
|
||||
|
|
|
|||
|
|
@ -39,14 +39,15 @@ test("publicModelId mirrors what /status reports for a path-loaded model", () =>
|
|||
});
|
||||
|
||||
test("a resident path-loaded model is matched by the id /status reports", () => {
|
||||
// A loose .gguf: the catalog row is keyed by the path, /status by the stem.
|
||||
// A loose .gguf: the catalog row is keyed by the path, and the Hub page records
|
||||
// the loadable identifier (status.model_identifier), so the literal pass answers.
|
||||
assert.equal(
|
||||
modelIdsMatch("Qwen3-8B-Q4_K_M", "/srv/models/Qwen3-8B-Q4_K_M.gguf"),
|
||||
false,
|
||||
);
|
||||
assert.equal(
|
||||
residentModelIdMatches(
|
||||
"Qwen3-8B-Q4_K_M",
|
||||
"/srv/models/Qwen3-8B-Q4_K_M.gguf",
|
||||
"/srv/models/Qwen3-8B-Q4_K_M.gguf",
|
||||
"/srv/models/Qwen3-8B-Q4_K_M.gguf",
|
||||
),
|
||||
|
|
@ -92,6 +93,37 @@ test("a resident path-loaded model is matched by the id /status reports", () =>
|
|||
assert.equal(residentModelIdMatches("Qwen3-8B-Q4_K_M"), false);
|
||||
});
|
||||
|
||||
test("a shared filename or folder name never marks a row resident", () => {
|
||||
// Two loose GGUFs with the same filename in different folders collapse onto one
|
||||
// public id, so a stem can only say "one of these", never which.
|
||||
const loaded = "/srv/models/alpha/model.gguf";
|
||||
const other = "/srv/models/beta/model.gguf";
|
||||
assert.equal(publicModelId(loaded), publicModelId(other));
|
||||
assert.equal(residentModelIdMatches(publicModelId(loaded), other, other), false);
|
||||
// The loadable identifier names exactly one of them.
|
||||
assert.equal(residentModelIdMatches(loaded, loaded, loaded), true);
|
||||
assert.equal(residentModelIdMatches(loaded, other, other), false);
|
||||
|
||||
// Same collapse one level up: two model directories sharing a basename.
|
||||
const loadedDir = "/srv/lmstudio/publisher-a/Llama-3-8B-GGUF";
|
||||
const otherDir = "/srv/models/publisher-b/Llama-3-8B-GGUF";
|
||||
assert.equal(publicModelId(loadedDir), publicModelId(otherDir));
|
||||
assert.equal(
|
||||
residentModelIdMatches(publicModelId(loadedDir), otherDir, otherDir),
|
||||
false,
|
||||
);
|
||||
|
||||
// A cache snapshot still collapses onto its repo id, which names one model.
|
||||
assert.equal(
|
||||
residentModelIdMatches(
|
||||
"unsloth/Qwen3-8B-GGUF",
|
||||
"/mnt/old-cache/models--unsloth--Qwen3-8B-GGUF/snapshots/abc123",
|
||||
null,
|
||||
),
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
test("Ollama link paths are recognised the way the resolver excludes them", () => {
|
||||
// core/inference/local_model_resolver.py refuses any path with these segments.
|
||||
assert.equal(
|
||||
|
|
|
|||
|
|
@ -695,8 +695,8 @@ def test_backfill_compares_server_keys_by_normalized_identity():
|
|||
src = " ".join(_read("features/model-picker/api/migrate-model-overrides.ts").split())
|
||||
assert "function normalizedOverrideKey(" in src
|
||||
# Folded on both sides: the older `id::variant` local keys are not.
|
||||
assert "const known = new Set(Object.keys(existing).map(normalizedOverrideKey));" in src
|
||||
assert "if (known.has(key)) { continue; }" in src
|
||||
assert "known.set(normalizedOverrideKey(storedKey), storedEntry);" in src
|
||||
assert "const stored = known.get(key);" in src
|
||||
# A quant-aware split, so a Windows drive letter is not read as a separator.
|
||||
assert "const split = splitQuantSuffix(key);" in src
|
||||
# Repo ids fold and POSIX paths do not, which is what these do.
|
||||
|
|
@ -957,27 +957,42 @@ def test_the_chat_picker_marks_ollama_targets_unloadable_by_the_api():
|
|||
assert 'seg in (".studio_links", "ollama_links")' in resolver, "the rule this mirrors"
|
||||
|
||||
|
||||
def test_the_backfill_writes_are_creates_not_replacements():
|
||||
def test_the_backfill_fills_in_fields_rather_than_skipping_known_keys():
|
||||
"""The backfill reads the override map once and then writes each model in turn,
|
||||
so a save by another tab during that pass was overwritten by this browser's
|
||||
older localStorage copy. The server tests and writes under one transaction
|
||||
rather than this re-fetching per model."""
|
||||
older localStorage copy. The server reads and writes under one transaction
|
||||
rather than this re-fetching per model, and it does so field by field: the
|
||||
override map shipped before this browser mirror did, holding only
|
||||
llama_extra_args and max_seq_length, so an entry-level skip would strand the
|
||||
context, KV cache, speculative and GPU settings this migration exists to carry."""
|
||||
backfill = " ".join(_read("features/model-picker/api/migrate-model-overrides.ts").split())
|
||||
assert "{ onlyIfAbsent: true }," in backfill
|
||||
assert "{ fillAbsentFields: true }," in backfill
|
||||
# Key presence alone is not "done": what the server lacks decides.
|
||||
assert "const stored = known.get(key);" in backfill
|
||||
assert "if (stored && absentFields(stored, current.config).length === 0) { continue; }" in backfill
|
||||
assert "const fields = Object.keys(toApiOverride(config));" in backfill
|
||||
assert "return fields.filter((field) => !(field in stored));" in backfill
|
||||
|
||||
api = " ".join(_read("features/model-picker/api/model-overrides.ts").split())
|
||||
assert "onlyIfAbsent?: boolean;" in api
|
||||
assert "options?.onlyIfAbsent ? { only_if_absent: true } : {}" in api.replace(
|
||||
assert "fillAbsentFields?: boolean;" in api
|
||||
assert "options?.fillAbsentFields ? { fill_absent_fields: true } : {}" in api.replace(
|
||||
"// biome-ignore lint/style/useNamingConvention: API schema ", ""
|
||||
)
|
||||
# Ordinary saves must stay unconditional, or a settings edit would never land.
|
||||
assert "syncModelOverride" in api and "only_if_absent: true" in api
|
||||
assert "syncModelOverride" in api and "fill_absent_fields: true" in api
|
||||
|
||||
route = (WORKDIR / "studio" / "backend" / "routes" / "settings.py").read_text(encoding = "utf-8")
|
||||
assert "only_if_absent: bool = False" in route, "the rule this mirrors"
|
||||
assert "only_if_absent = payload.only_if_absent," in route
|
||||
assert "fill_absent_fields: bool = False" in route, "the rule this mirrors"
|
||||
assert "fill_absent_fields = payload.fill_absent_fields," in route
|
||||
# A write mode must not leak into the saved fields, or "only model_id means
|
||||
# forget this model" stops working.
|
||||
assert '"remove", "only_if_absent"' in route
|
||||
assert '"remove", "fill_absent_fields"' in route
|
||||
|
||||
# The merge is the server's, under the write's own transaction: a client-side
|
||||
# read-modify-write would reopen the race the conditional write closed.
|
||||
db = (WORKDIR / "studio" / "backend" / "storage" / "studio_db.py").read_text(encoding = "utf-8")
|
||||
assert "merged = {**entry_value, **stored}" in db
|
||||
assert "BEGIN IMMEDIATE" in db
|
||||
|
||||
|
||||
def test_the_hub_settings_page_matches_a_resident_path_loaded_model():
|
||||
|
|
@ -992,10 +1007,21 @@ def test_the_hub_settings_page_matches_a_resident_path_loaded_model():
|
|||
)
|
||||
assert "loadedConfig={settingsTargetIsResident ? activeModelConfig : null}" in hub
|
||||
assert "settingsTargetIsResident ? activeGgufContextLength : null" in hub
|
||||
# The loadable identifier, as every other status reader records it. active_model
|
||||
# is the clean public id, and two files sharing a filename collapse onto one, so
|
||||
# storing it would let the wrong catalog row look loaded.
|
||||
assert "const checkpointId = resolveInferenceCheckpointId(status);" in hub
|
||||
assert "store.setCheckpoint(checkpointId, status.gguf_variant ?? null);" in hub
|
||||
assert "setCheckpoint(status.active_model" not in hub
|
||||
chat = " ".join(_read("features/chat/lib/apply-inference-status-to-store.ts").split())
|
||||
assert "return status.model_identifier ?? status.active_model;" in chat, "the rule this mirrors"
|
||||
# The alias is the backend's own public id rule, not a private heuristic.
|
||||
identity = _read("features/hub/lib/model-identity.ts")
|
||||
assert "export function publicModelId(" in identity
|
||||
assert "models--" in identity and "snapshots" in identity
|
||||
# Only a namespaced repo id names one model; a filename or directory stem does
|
||||
# not, so it must never stand in for the loaded model's identity.
|
||||
assert 'return publicId.includes("/") && modelIdsMatch(active, publicId);' in identity
|
||||
backend = (WORKDIR / "studio" / "backend" / "core" / "inference" / "model_ids.py").read_text(
|
||||
encoding = "utf-8"
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue