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. */