From baf11a01f7030c01a761ca119b82a91729f735e9 Mon Sep 17 00:00:00 2001 From: Unsloth Date: Sun, 26 Jul 2026 23:31:51 -0700 Subject: [PATCH] studio: clear server overrides for models evicted from local storage Saving a config can push the browser's map over its entry or byte budget, at which point older models are silently dropped and the save still reports success. Their server-side overrides survived, so API loads kept applying settings that nothing in the UI showed any more and nothing could forget. savePerModelConfig now reports what it evicted, and the caller clears those server entries alongside the one it saved. Removal also had to resolve the way lookup does. Storage keys are normalized, so an evicted entry comes back lowercased while the server may hold the repo's real casing; deleting only the literal key would leave the entry a load still resolves to. Read and remove now share resolve_model_override_key, so what a load applies and what forgetting clears cannot disagree. Path ids still match exactly, so one file is never forgotten by clearing its case-variant neighbour. --- studio/backend/routes/settings.py | 7 ++++- .../backend/tests/test_openai_auto_switch.py | 31 +++++++++++++++++++ .../utils/openai_auto_switch_settings.py | 28 ++++++++++++----- .../components/model-config-page.tsx | 8 +++++ .../model-config/per-model-config.ts | 30 +++++++++++++++--- 5 files changed, 91 insertions(+), 13 deletions(-) diff --git a/studio/backend/routes/settings.py b/studio/backend/routes/settings.py index f3b40bdcdb..d0ff58d944 100644 --- a/studio/backend/routes/settings.py +++ b/studio/backend/routes/settings.py @@ -43,6 +43,7 @@ from utils.openai_auto_switch_settings import ( get_auto_unload_keep_kv, get_model_overrides, get_openai_auto_switch_enabled, + resolve_model_override_key, get_stored_auto_unload_idle_seconds, set_model_override, set_openai_auto_switch, @@ -394,7 +395,11 @@ def update_openai_auto_switch_override( # form field must not turn "forget this model" into an update that # keeps it. Only the explicit flag short-circuits; the legacy # inferred path still just gates launch-flag carry-over. - set_model_override(payload.model_id, llama_extra_args = [], max_seq_length = None) + # Remove the key a load would actually resolve to, not just the + # literal one sent: the browser normalizes casing before storing, so + # the two can differ and a stale entry would survive forgetting. + target_id = resolve_model_override_key(payload.model_id) or payload.model_id + set_model_override(target_id, llama_extra_args = [], max_seq_length = None) else: set_model_override( payload.model_id, diff --git a/studio/backend/tests/test_openai_auto_switch.py b/studio/backend/tests/test_openai_auto_switch.py index bfd1754301..b8720cb440 100644 --- a/studio/backend/tests/test_openai_auto_switch.py +++ b/studio/backend/tests/test_openai_auto_switch.py @@ -4404,3 +4404,34 @@ def test_a_non_gpu_load_failure_is_not_retried(monkeypatch): with pytest.raises(HTTPException): _run_hook("unsloth/B-GGUF") assert calls["n"] == 1 + + +def test_removal_clears_the_entry_a_load_would_actually_resolve(monkeypatch): + # The browser normalizes casing before storing, so a forget request can carry + # a different casing than the stored key. Removing only the literal key would + # leave the entry a load still resolves to, with no UI able to clear it. + import routes.settings as settings_route + + _mock_override_store(monkeypatch) + settings.set_model_override("unsloth/B-GGUF:Q4_K_M", max_seq_length = 8192) + assert settings.get_model_override("unsloth/b-gguf:q4_k_m")["max_seq_length"] == 8192 + + settings_route.update_openai_auto_switch_override( + settings_route.ModelOverridePayload(model_id = "unsloth/b-gguf:q4_k_m", remove = True), + "tester", + ) + assert settings.get_model_overrides() == {} + assert settings.get_model_override("unsloth/B-GGUF:Q4_K_M") == {} + + +def test_removal_of_a_path_still_only_touches_the_exact_key(monkeypatch): + import routes.settings as settings_route + + _mock_override_store(monkeypatch) + settings.set_model_override("/models/foo.gguf", max_seq_length = 8192) + settings_route.update_openai_auto_switch_override( + settings_route.ModelOverridePayload(model_id = "/models/Foo.gguf", remove = True), + "tester", + ) + # A different file must survive its neighbour being forgotten. + assert settings.get_model_override("/models/foo.gguf")["max_seq_length"] == 8192 diff --git a/studio/backend/utils/openai_auto_switch_settings.py b/studio/backend/utils/openai_auto_switch_settings.py index 830d3cd3f7..2a5844043d 100644 --- a/studio/backend/utils/openai_auto_switch_settings.py +++ b/studio/backend/utils/openai_auto_switch_settings.py @@ -450,24 +450,36 @@ def get_model_override(model_id: str) -> dict: an ambiguous fallback matches nothing, so two POSIX paths differing only in case stay distinct. """ - overrides = get_model_overrides() - override = overrides.get(model_id) - if isinstance(override, dict): - return override - if not isinstance(model_id, str): + key = resolve_model_override_key(model_id) + if key is None: return {} + override = get_model_overrides().get(key) + return override if isinstance(override, dict) else {} + + +def resolve_model_override_key(model_id: str) -> Optional[str]: + """The stored key an override lookup for ``model_id`` would actually hit. + + Shared by read and remove so "what a load applies" and "what forgetting this + model clears" can never disagree. + """ + overrides = get_model_overrides() + if isinstance(overrides.get(model_id), dict): + return model_id + if not isinstance(model_id, str): + return None # Only repo-style ids fold. A POSIX path is case-sensitive and names a # different file, so matching "/models/Foo.gguf" against an entry saved for # "/models/foo.gguf" would replay another model's context and GPU pin. if _looks_like_filesystem_path(model_id): - return {} + return None folded = model_id.casefold() matches = [ - value + key for key, value in overrides.items() if isinstance(key, str) and key.casefold() == folded and isinstance(value, dict) ] - return matches[0] if len(matches) == 1 else {} + return matches[0] if len(matches) == 1 else None def set_model_override( diff --git a/studio/frontend/src/features/model-picker/components/model-config-page.tsx b/studio/frontend/src/features/model-picker/components/model-config-page.tsx index 91e71d8272..ec02532217 100644 --- a/studio/frontend/src/features/model-picker/components/model-config-page.tsx +++ b/studio/frontend/src/features/model-picker/components/model-config-page.tsx @@ -869,11 +869,13 @@ export function ModelConfigPage({ isActiveModel && effectiveAtBaseline && rememberChanged; const defaultConfig = isDefaultConfig(effectiveRuntimeConfig); let saveFailed = false; + const evicted: { modelId: string; ggufVariant: string | null }[] = []; if (remember) { saveFailed = !savePerModelConfig( target.id, target.ggufVariant, effectiveRuntimeConfig, + evicted, ); } else { saveFailed = !deletePerModelConfig(target.id, target.ggufVariant); @@ -896,6 +898,12 @@ export function ModelConfigPage({ remember ? effectiveRuntimeConfig : null, ); } + // Saving can push the local map over budget and silently drop other models. + // Their server entries would otherwise keep being applied by API loads with + // nothing left in the UI showing them or able to forget them. + for (const dropped of evicted) { + syncModelOverride(dropped.modelId, dropped.ggufVariant, null); + } if (effectivePersistenceOnly) { if (saveFailed) { toast.error("Couldn't save settings for this model."); diff --git a/studio/frontend/src/features/model-picker/model-config/per-model-config.ts b/studio/frontend/src/features/model-picker/model-config/per-model-config.ts index 8511f3d58e..acc8efdefa 100644 --- a/studio/frontend/src/features/model-picker/model-config/per-model-config.ts +++ b/studio/frontend/src/features/model-picker/model-config/per-model-config.ts @@ -217,6 +217,7 @@ function serializedMapEntrySize(key: string, value: StoredMap[string]): number { function deleteOldestEvictableEntry( map: StoredMap, protectedKeys?: ReadonlySet, + evicted?: string[], ): { key: string; value: StoredMap[string] } | null { for (const key of Object.keys(map)) { // Never evict a future-schema entry an older client cannot interpret. @@ -228,6 +229,7 @@ function deleteOldestEvictableEntry( } const value = map[key]; delete map[key]; + evicted?.push(key); return { key, value }; } return null; @@ -236,17 +238,18 @@ function deleteOldestEvictableEntry( function enforceStorageBudget( map: StoredMap, protectedKeys?: ReadonlySet, + evicted?: string[], ): boolean { let entryCount = Object.keys(map).length; while (entryCount > MAX_ENTRIES) { - if (!deleteOldestEvictableEntry(map, protectedKeys)) { + if (!deleteOldestEvictableEntry(map, protectedKeys, evicted)) { return false; } entryCount -= 1; } let bytes = serializedMapSize(map); while (bytes > MAX_PER_MODEL_CONFIG_STORAGE_BYTES) { - const removed = deleteOldestEvictableEntry(map, protectedKeys); + const removed = deleteOldestEvictableEntry(map, protectedKeys, evicted); if (!removed) { return false; } @@ -623,6 +626,13 @@ export function savePerModelConfig( modelId: string, ggufVariant: string | null | undefined, config: PerModelConfig, + /** + * Receives models dropped to stay inside the storage budget. Eviction is + * silent and still reports success, so without this their server-side + * overrides would keep being applied by API loads with nothing in the UI + * still showing them or able to forget them. + */ + evicted?: { modelId: string; ggufVariant: string | null }[], ): boolean { if ( typeof config.chatTemplateOverride === "string" && @@ -646,10 +656,22 @@ export function savePerModelConfig( const [key] = storageKeysForModelVariant(modelId, ggufVariant); deleteConfigEntriesForModelVariant(map, modelId, ggufVariant); map[key] = toStoredConfig(normalized); - if (!enforceStorageBudget(map, new Set([key]))) { + const evictedKeys: string[] = []; + if (!enforceStorageBudget(map, new Set([key]), evictedKeys)) { return false; } - return writeMap(map); + const written = writeMap(map); + if (written && evicted) { + for (const evictedKey of evictedKeys) { + const id = modelIdFromStorageKey(evictedKey); + if (!id) { + continue; + } + const variant = ggufVariantFromStorageKey(evictedKey); + evicted.push({ modelId: id, ggufVariant: variant ? variant : null }); + } + } + return written; } /** Every saved per-model config, decoded back to the ids it was keyed by. */