diff --git a/studio/backend/routes/inference.py b/studio/backend/routes/inference.py index beb9ebdb87..d804b3143d 100644 --- a/studio/backend/routes/inference.py +++ b/studio/backend/routes/inference.py @@ -3747,7 +3747,9 @@ async def _maybe_auto_switch_model( ) ) saved_gpu_ids = load_kwargs.get("gpu_ids") - if saved_gpu_ids and not _override_gpu_ids_still_resolve(saved_gpu_ids): + if saved_gpu_ids and not await _override_gpu_ids_still_resolve( + saved_gpu_ids + ): # A pin saved before a GPU was removed, before a # visibility-mask change, or on another host. Dropping the # one dead field beats 400ing the whole load. @@ -3977,11 +3979,13 @@ def _classify_diffusion_gguf(config: ModelConfig) -> Optional[bool]: return True if name_says_diffusion else None -def _override_gpu_ids_still_resolve(gpu_ids: List[int]) -> bool: +async def _override_gpu_ids_still_resolve(gpu_ids: List[int]) -> bool: """Whether a per-model GPU pin is usable on this machine right now. normalize_model_override cannot know the device list, so it stores whatever - was valid where the config was written. This is the load-time reconciliation. + was valid where the config was written. This is the load-time reconciliation, + and it has to make every check _resolve_gguf_gpu_ids_for_request would later + make, or the load 400s on the check this one skipped. """ try: from utils.hardware import DeviceType, get_device @@ -3991,7 +3995,18 @@ def _override_gpu_ids_still_resolve(gpu_ids: List[int]) -> bool: if get_device() == DeviceType.XPU and not is_vulkan: # gpu_ids is rejected outright on XPU. return False - resolve_requested_gpu_ids(gpu_ids, is_vulkan = is_vulkan) + resolved = resolve_requested_gpu_ids(gpu_ids, is_vulkan = is_vulkan) + if is_vulkan and resolved: + # Vulkan ordinals are their own index space, so resolve() only rejects + # malformed ones. Presence needs the same ggml probe the load does. + binary = LlamaCppBackend._find_llama_server_binary() + if binary: + probed = { + gpu[0] + for gpu in await asyncio.to_thread(LlamaCppBackend._get_gpu_memory, binary) + } + if not {int(gpu_id) for gpu_id in resolved}.issubset(probed): + return False return True except Exception: return False diff --git a/studio/backend/routes/settings.py b/studio/backend/routes/settings.py index 07a751cf61..c6f64f8f16 100644 --- a/studio/backend/routes/settings.py +++ b/studio/backend/routes/settings.py @@ -158,6 +158,10 @@ class ModelOverridePayload(BaseModel): gpu_layers: Optional[int] = Field(default = None, ge = -1, le = 1024) n_cpu_moe: Optional[int] = Field(default = None, ge = 0, le = 1024) gpu_ids: Optional[list[int]] = None + # Explicit intent. A save whose config is entirely default carries no fields + # at all, which is indistinguishable from "forget this model" by shape alone. + # None keeps the original contract: a bare model_id means remove. + remove: Optional[bool] = None @field_validator("chat_template_override") @classmethod @@ -354,11 +358,14 @@ def update_openai_auto_switch_override( # them and must not delete them). requested_extra_args = payload.llama_extra_args saved_fields = payload.model_dump( - exclude = {"model_id", "llama_extra_args"}, exclude_none = True + exclude = {"model_id", "llama_extra_args", "remove"}, exclude_none = True ) - is_removal = not payload.tensor_parallel and not { - key: value for key, value in saved_fields.items() if key != "tensor_parallel" - } + if payload.remove is not None: + is_removal = payload.remove + else: + is_removal = not payload.tensor_parallel and not { + 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: diff --git a/studio/backend/tests/test_openai_auto_switch.py b/studio/backend/tests/test_openai_auto_switch.py index 503604f03f..fc02f3f538 100644 --- a/studio/backend/tests/test_openai_auto_switch.py +++ b/studio/backend/tests/test_openai_auto_switch.py @@ -4091,7 +4091,11 @@ def test_stale_gpu_ids_are_dropped_not_fatal(monkeypatch): "get_model_override", lambda mid: {"gpu_ids": [0, 1], "max_seq_length": 4096}, ) - monkeypatch.setattr(inference_route, "_override_gpu_ids_still_resolve", lambda ids: False) + + async def _unusable(ids): + return False + + monkeypatch.setattr(inference_route, "_override_gpu_ids_still_resolve", _unusable) _run_hook("unsloth/B-GGUF") req = rec.calls[0] @@ -4111,7 +4115,11 @@ def test_usable_gpu_ids_are_kept(monkeypatch): recorder = rec, ) monkeypatch.setattr(settings, "get_model_override", lambda mid: {"gpu_ids": [0, 1]}) - monkeypatch.setattr(inference_route, "_override_gpu_ids_still_resolve", lambda ids: True) + + async def _usable(ids): + return True + + monkeypatch.setattr(inference_route, "_override_gpu_ids_still_resolve", _usable) _run_hook("unsloth/B-GGUF") assert rec.calls[0].gpu_ids == [0, 1] @@ -4126,4 +4134,89 @@ def test_override_gpu_ids_probe_never_raises(monkeypatch): raise RuntimeError("driver exploded") monkeypatch.setattr(hw, "resolve_requested_gpu_ids", boom) - assert inference_route._override_gpu_ids_still_resolve([0]) is False + assert asyncio.run(inference_route._override_gpu_ids_still_resolve([0])) is False + + +def test_vulkan_ordinal_absent_from_the_probe_is_unusable(monkeypatch): + # resolve_requested_gpu_ids only rejects malformed Vulkan ordinals, so + # presence needs the same ggml probe the load itself runs. Without it this + # helper says "fine" and the load 400s on the check it skipped. + from core.inference.llama_cpp import LlamaCppBackend + + monkeypatch.setattr(LlamaCppBackend, "_is_vulkan_backend", staticmethod(lambda: True)) + monkeypatch.setattr( + LlamaCppBackend, "_find_llama_server_binary", staticmethod(lambda: "/bin/llama-server") + ) + monkeypatch.setattr( + LlamaCppBackend, "_get_gpu_memory", staticmethod(lambda binary: [(0, 8192)]) + ) + assert asyncio.run(inference_route._override_gpu_ids_still_resolve([0])) is True + assert asyncio.run(inference_route._override_gpu_ids_still_resolve([7])) is False + assert asyncio.run(inference_route._override_gpu_ids_still_resolve([0, 1])) is False + + +def test_vulkan_probe_without_a_binary_does_not_block_the_load(monkeypatch): + # No binary means nothing to probe with. Refusing here would drop a valid + # pin on every load, so the later path stays the authority. + from core.inference.llama_cpp import LlamaCppBackend + + monkeypatch.setattr(LlamaCppBackend, "_is_vulkan_backend", staticmethod(lambda: True)) + monkeypatch.setattr(LlamaCppBackend, "_find_llama_server_binary", staticmethod(lambda: None)) + assert asyncio.run(inference_route._override_gpu_ids_still_resolve([0])) is True + + +def test_default_save_preserves_flags_instead_of_removing(monkeypatch): + # "Remember for this model" is on but every value is default, so the payload + # carries no fields. That is shape-identical to a removal, and guessing wrong + # wipes launch flags no UI can show or restore. + import routes.settings as settings_route + + _mock_override_store(monkeypatch) + settings.set_model_override("unsloth/B-GGUF", llama_extra_args = ["--flash-attn"]) + + resp = settings_route.update_openai_auto_switch_override( + settings_route.ModelOverridePayload(model_id = "unsloth/B-GGUF", remove = False), + "tester", + ) + assert resp.overrides["unsloth/B-GGUF"]["llama_extra_args"] == ["--flash-attn"] + + +def test_explicit_remove_still_clears_everything(monkeypatch): + import routes.settings as settings_route + + _mock_override_store(monkeypatch) + settings.set_model_override( + "unsloth/B-GGUF", llama_extra_args = ["--flash-attn"], max_seq_length = 4096 + ) + resp = settings_route.update_openai_auto_switch_override( + settings_route.ModelOverridePayload( + model_id = "unsloth/B-GGUF", remove = True, llama_extra_args = [] + ), + "tester", + ) + assert "unsloth/B-GGUF" not in resp.overrides + + +def test_bare_payload_without_remove_flag_still_removes(monkeypatch): + # The original contract, kept for any caller that predates the flag. + import routes.settings as settings_route + + _mock_override_store(monkeypatch) + settings.set_model_override("unsloth/B-GGUF", max_seq_length = 4096) + resp = settings_route.update_openai_auto_switch_override( + settings_route.ModelOverridePayload(model_id = "unsloth/B-GGUF"), "tester" + ) + assert "unsloth/B-GGUF" not in resp.overrides + + +def test_remove_false_with_real_fields_saves_normally(monkeypatch): + import routes.settings as settings_route + + _mock_override_store(monkeypatch) + resp = settings_route.update_openai_auto_switch_override( + settings_route.ModelOverridePayload( + model_id = "unsloth/B-GGUF", remove = False, max_seq_length = 8192 + ), + "tester", + ) + assert resp.overrides["unsloth/B-GGUF"]["max_seq_length"] == 8192 diff --git a/studio/frontend/src/app/routes/__root.tsx b/studio/frontend/src/app/routes/__root.tsx index 5dbeefa8f8..ff7de58a4f 100644 --- a/studio/frontend/src/app/routes/__root.tsx +++ b/studio/frontend/src/app/routes/__root.tsx @@ -3,25 +3,26 @@ import { AppSidebar } from "@/components/app-sidebar"; import { Navbar } from "@/components/navbar"; -import { fetchDeviceType, usePlatformStore } from "@/config/env"; import { SidebarInset, SidebarProvider } from "@/components/ui/sidebar"; -import { SettingsDialog, useSettingsDialogStore } from "@/features/settings"; +import { fetchDeviceType, usePlatformStore } from "@/config/env"; +import { ApiMonitorOverlay } from "@/features/api-monitor/api-monitor-overlay"; +import { hasAuthToken } from "@/features/auth"; import { ChatPage, + type ChatSearch, clearNewChatDraft, useChatRuntimeStore, - type ChatSearch, } from "@/features/chat"; -import { ApiMonitorOverlay } from "@/features/api-monitor/api-monitor-overlay"; -import { RemoteCodeConsentDialog } from "@/features/security"; -import { HfTokenWarningDialog } from "@/features/hf-auth"; -import { TransformersUpgradeDialog } from "@/features/transformers-upgrade"; -import { useTrainingUnloadGuard } from "@/features/training"; import { useExportRuntimeLifecycle } from "@/features/export"; -import { hasAuthToken } from "@/features/auth"; +import { HfTokenWarningDialog } from "@/features/hf-auth"; +import { backfillModelOverrides } from "@/features/model-picker/api/migrate-model-overrides"; import { usePersonalizationSync } from "@/features/profile"; +import { RemoteCodeConsentDialog } from "@/features/security"; +import { SettingsDialog, useSettingsDialogStore } from "@/features/settings"; +import { useTrainingUnloadGuard } from "@/features/training"; +import { TransformersUpgradeDialog } from "@/features/transformers-upgrade"; import { useSidebarPin } from "@/hooks/use-sidebar-pin"; -import { useT, type TranslationKey } from "@/i18n"; +import { type TranslationKey, useT } from "@/i18n"; import { Outlet, createRootRoute, @@ -174,6 +175,16 @@ function RootLayout() { : DEFAULT_DOCUMENT_TITLE; }, [documentTitle]); + // Settings saved before the server-side override map existed live only in this + // browser, so an API load would use app defaults while the UI still showed the + // model as remembered. Backfill once, after auth. + useEffect(() => { + if (isAuthFlowRoute) { + return; + } + void backfillModelOverrides(); + }, [isAuthFlowRoute]); + useEffect(() => { if (isAuthFlowRoute) { useSettingsDialogStore.getState().closeDialog(); diff --git a/studio/frontend/src/features/api-monitor/api-monitor-overlay.tsx b/studio/frontend/src/features/api-monitor/api-monitor-overlay.tsx index 4411e052ad..88a4915b93 100644 --- a/studio/frontend/src/features/api-monitor/api-monitor-overlay.tsx +++ b/studio/frontend/src/features/api-monitor/api-monitor-overlay.tsx @@ -111,7 +111,9 @@ export function ApiMonitorOverlay(): ReactElement | null { // One loop for both jobs: panel contents while open, traffic watch while // closed. Stands down on the full page, which polls for itself. useEffect(() => { - if (onFullPage) { + // Opted out and closed: the panel can neither open nor show anything, so + // polling would be pure background load on every open Studio window. + if (onFullPage || (!autoOpen && !isOpen)) { return; } let cancelled = false; @@ -146,7 +148,7 @@ export function ApiMonitorOverlay(): ReactElement | null { cancelled = true; if (timer !== undefined) window.clearTimeout(timer); }; - }, [isOpen, onFullPage]); + }, [isOpen, onFullPage, autoOpen]); const entries = useMemo(() => data?.entries ?? [], [data]); const stats = useMemo(() => computeStats(entries), [entries]); diff --git a/studio/frontend/src/features/model-picker/api/migrate-model-overrides.ts b/studio/frontend/src/features/model-picker/api/migrate-model-overrides.ts new file mode 100644 index 0000000000..945dc1fbdd --- /dev/null +++ b/studio/frontend/src/features/model-picker/api/migrate-model-overrides.ts @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: AGPL-3.0-only +// Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 + +// One-time backfill of per-model settings into the server override map. +// +// Settings used to live only in this browser, so on upgrade the server knows +// nothing about models the user already configured. Without this they keep +// showing as remembered in the UI while an API load quietly uses app defaults, +// which is the exact bug the server-side map exists to fix. + +import { + isDefaultConfig, + listPerModelConfigs, +} from "../model-config/per-model-config"; +import { + fetchModelOverrides, + modelOverrideKey, + putModelOverride, +} from "./model-overrides"; + +const DONE_FLAG = "unsloth_model_overrides_backfilled_v1"; + +function alreadyRan(): boolean { + try { + return window.localStorage.getItem(DONE_FLAG) === "1"; + } catch { + // Storage denied: treat as done rather than re-running on every mount. + return true; + } +} + +function markRan(): void { + try { + window.localStorage.setItem(DONE_FLAG, "1"); + } catch { + // Nothing to do; the backfill is idempotent anyway. + } +} + +/** + * Push local configs the server has never seen. Never deletes and never + * overwrites: an entry already on the server is the newer authority, and losing + * a setting here would be worse than leaving one unmigrated. + */ +export async function backfillModelOverrides(): Promise { + if (alreadyRan()) { + return; + } + const local = listPerModelConfigs().filter( + (entry) => !isDefaultConfig(entry.config), + ); + if (local.length === 0) { + markRan(); + return; + } + + let existing: Awaited>; + try { + existing = await fetchModelOverrides(); + } catch { + // Offline or not authenticated yet. Leave the flag unset so the next start + // tries again rather than silently skipping the migration forever. + return; + } + + let failed = false; + for (const entry of local) { + const key = modelOverrideKey(entry.modelId, entry.ggufVariant); + if (existing[key]) { + continue; + } + try { + await putModelOverride(entry.modelId, entry.ggufVariant, entry.config); + } catch { + failed = true; + } + } + if (!failed) { + markRan(); + } +} 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 e3a735884b..d4e89985b4 100644 --- a/studio/frontend/src/features/model-picker/api/model-overrides.ts +++ b/studio/frontend/src/features/model-picker/api/model-overrides.ts @@ -134,6 +134,10 @@ export async function putModelOverride( body: JSON.stringify({ // biome-ignore lint/style/useNamingConvention: API schema model_id: modelOverrideKey(modelId, ggufVariant), + // Say which operation this is. A save of an all-default config carries no + // fields, which is shape-identical to "forget this model", and guessing + // wrong wipes launch flags the UI cannot show or restore. + remove: config === null, // Launch flags have no UI control, so the backend preserves them when the // field is omitted. Forgetting a model means forgetting all of it, so that // path sends an explicit empty list to clear them. 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 ba6d4cec99..8511f3d58e 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 @@ -416,7 +416,10 @@ function writeMap(map: StoredMap): boolean { } } -function warnDroppedFields(raw: Record, version: number): void { +function warnDroppedFields( + raw: Record, + version: number, +): void { if (!import.meta.env?.DEV) { return; } @@ -436,7 +439,8 @@ function normalizeV1(partial: RawConfig): PerModelConfig { typeof partial.speculativeType === "string" ? canonicalizeSpeculativeType(partial.speculativeType) : null; - const speculativeType = rawSpecType ?? DEFAULT_PER_MODEL_CONFIG.speculativeType; + const speculativeType = + rawSpecType ?? DEFAULT_PER_MODEL_CONFIG.speculativeType; const specDraftNMax = speculativeType != null && MTP_SPECULATIVE_TYPES.has(speculativeType) && @@ -648,6 +652,32 @@ export function savePerModelConfig( return writeMap(map); } +/** Every saved per-model config, decoded back to the ids it was keyed by. */ +export function listPerModelConfigs(): { + modelId: string; + ggufVariant: string | null; + config: PerModelConfig; +}[] { + const out: { + modelId: string; + ggufVariant: string | null; + config: PerModelConfig; + }[] = []; + for (const [key, raw] of Object.entries(readMap())) { + const modelId = modelIdFromStorageKey(key); + if (!modelId) { + continue; + } + const variant = ggufVariantFromStorageKey(key); + out.push({ + modelId, + ggufVariant: variant ? variant : null, + config: normalize(raw), + }); + } + return out; +} + export function deletePerModelConfig( modelId: string, ggufVariant?: string | null,