From 00c4ef762e0bb6112517b84d01ec13468bcb97be Mon Sep 17 00:00:00 2001 From: LeoBorcherding Date: Thu, 23 Jul 2026 23:00:31 -0500 Subject: [PATCH] GPU pick index-space: stamp the kind onto the pick, not the call site The fromPersisted-by-call-site marker kept getting misapplied because those sites carry both storage restores and fresh live edits, and the cold-cache park/revalidate raced the load path. Replace it with per-value provenance: each GPU pick carries the index space it was made under, and reconcile drops it only when that stamp no longer matches the backend. - use-gpu-info: currentGpuIndexKind() (from /api/system gguf_devices); drop the session-global gpuIndexSpaceChanged marker machinery. - PerModelConfig gains selectedGpuIdsIndexKind (persisted); the runtime store gains selectedGpuIdsKind, stamped wherever a pick is set (live picker = current kind, load echo = current kind, config apply = the config's stamp). - reconcilePersistedGpuIds(ids, savedKind): drop when savedKind != current kind (both known); unstamped picks default to physical (pre-Vulkan). - Run settings stamps the current kind when the user edits the GPU picker, so an edited pick is never treated as a stale cross-space restore; an unedited stored pick keeps its saved kind. - No more park-null / async revalidate / fromPersisted: a same-backend refresh keeps the saved subset (kinds match), a cross-space pin is dropped, and the load snapshot carries the stamp so it can't send a wrong-space pin. Fixes the three Codex findings (cold-cache saved-pick loss, model-generation race, unedited run-settings treated as live). Contract tests updated; typecheck + build + tests/studio contracts green. --- .../src/features/chat/api/chat-adapter.ts | 11 ++- .../frontend/src/features/chat/chat-page.tsx | 4 -- .../chat/hooks/use-chat-model-runtime.ts | 3 + .../src/features/chat/shared-composer.tsx | 27 ++++--- .../chat/stores/chat-runtime-store.ts | 43 ++++++++--- studio/frontend/src/features/hub/hub-page.tsx | 5 +- .../components/model-config-page.tsx | 12 +++- .../model-config/apply-per-model-config.ts | 71 ++++++------------- .../model-config/per-model-config.ts | 19 +++++ studio/frontend/src/hooks/use-gpu-info.ts | 36 ++++------ tests/studio/test_model_picker_contracts.py | 52 +++++++++----- 11 files changed, 162 insertions(+), 121 deletions(-) diff --git a/studio/frontend/src/features/chat/api/chat-adapter.ts b/studio/frontend/src/features/chat/api/chat-adapter.ts index b14efca52d..2054384e5a 100644 --- a/studio/frontend/src/features/chat/api/chat-adapter.ts +++ b/studio/frontend/src/features/chat/api/chat-adapter.ts @@ -1562,9 +1562,14 @@ async function autoLoadSmallestModel(): Promise<{ } const effectiveGpuIds = config.selectedGpuIds !== undefined - ? reconcilePersistedGpuIds(config.selectedGpuIds, { - fromPersisted: true, - }) + ? reconcilePersistedGpuIds( + config.selectedGpuIds, + // Drop a saved pick whose stamped index space no longer matches the + // current backend; a pre-stamp stored pick defaults to physical. + config.selectedGpuIds == null + ? null + : (config.selectedGpuIdsIndexKind ?? "physical"), + ) : null; // Under Manual GPU memory + Auto layers, llama.cpp's --fit owns context // sizing, so send 0 (or the pinned length). GGUF-only; a no-op otherwise. diff --git a/studio/frontend/src/features/chat/chat-page.tsx b/studio/frontend/src/features/chat/chat-page.tsx index 71c304deac..e46ea0ac46 100644 --- a/studio/frontend/src/features/chat/chat-page.tsx +++ b/studio/frontend/src/features/chat/chat-page.tsx @@ -2323,9 +2323,6 @@ export function ChatPage({ }); const hasAppliedConfig = applyModelLoadConfigToRuntime( selection.config ?? rememberedConfigFor(selection), - // Only the rememberedConfigFor fallback is a storage restore; an - // explicit selection.config is a fresh pick in the current index space. - { fromPersisted: !selection.config }, ); await selectModel({ ...selection, @@ -2996,7 +2993,6 @@ export function ChatPage({ }); const hasAppliedConfig = applyModelLoadConfigToRuntime( rememberedConfigFor(selection), - { fromPersisted: true }, ); await selectModelRef.current({ ...selection, diff --git a/studio/frontend/src/features/chat/hooks/use-chat-model-runtime.ts b/studio/frontend/src/features/chat/hooks/use-chat-model-runtime.ts index 76a310ac33..082f6d6ede 100644 --- a/studio/frontend/src/features/chat/hooks/use-chat-model-runtime.ts +++ b/studio/frontend/src/features/chat/hooks/use-chat-model-runtime.ts @@ -668,6 +668,9 @@ export function useChatModelRuntime() { } let loadSelectedGpuIds = reconcilePersistedGpuIds( stateBeforeUnload.selectedGpuIds, + // The snapshot carries the index space the pick is in, so a + // cross-space pick (after a llama.cpp backend swap) is dropped. + stateBeforeUnload.selectedGpuIdsKind, ); let loadSpeculativeType = stateBeforeUnload.speculativeType; let loadSpecDraftNMax = stateBeforeUnload.specDraftNMax; diff --git a/studio/frontend/src/features/chat/shared-composer.tsx b/studio/frontend/src/features/chat/shared-composer.tsx index 479e3a53fb..c8057dbf00 100644 --- a/studio/frontend/src/features/chat/shared-composer.tsx +++ b/studio/frontend/src/features/chat/shared-composer.tsx @@ -997,8 +997,13 @@ export function SharedComposer({ splitRatio: store.splitRatio, // Reconcile the pick against the GPUs present now, like the model-switch // path: an early remember-restore can hold a stale cross-host pick that - // /load would reject (the device cache is populated by send time). - selectedGpuIds: reconcilePersistedGpuIds(store.selectedGpuIds), + // /load would reject (the device cache is populated by send time). The + // store carries the index space the pick is in, so a cross-space pick + // (after a llama.cpp backend swap) is dropped here too. + selectedGpuIds: reconcilePersistedGpuIds( + store.selectedGpuIds, + store.selectedGpuIdsKind, + ), customContextLength: store.customContextLength, }; // Set when an accepted transformers install unloaded the active model @@ -1057,12 +1062,18 @@ export function SharedComposer({ ownConfig.nCpuMoe ?? compareLoadKnobs.nCpuMoe; const effectiveSelectedGpuIds = ownConfig.selectedGpuIds !== undefined - ? reconcilePersistedGpuIds(ownConfig.selectedGpuIds, { - // Only a resolveInitialConfig storage hit is a real restore; an - // explicit sel.config is a fresh current-space pane pick, so it - // must stay live and never take the index-space invalidation. - fromPersisted: !config && ownRemembered, - }) + ? reconcilePersistedGpuIds( + ownConfig.selectedGpuIds, + // The pick's index space is intrinsic to the config: a fresh + // sel.config pane pick carries the current kind (stamped on + // edit), a storage restore carries its saved kind, and a + // pre-stamp stored pick defaults to physical. So an explicit + // pane pick stays live while a cross-space restore is dropped -- + // no need to distinguish sel.config from a remembered config. + ownConfig.selectedGpuIds == null + ? null + : (ownConfig.selectedGpuIdsIndexKind ?? "physical"), + ) : compareLoadKnobs.selectedGpuIds; // A pane's context comes from its own config only: a saved pin, or null // (Auto/native). It must not inherit the active model's shared snapshot -- diff --git a/studio/frontend/src/features/chat/stores/chat-runtime-store.ts b/studio/frontend/src/features/chat/stores/chat-runtime-store.ts index 0c2dea61f8..b241891b63 100644 --- a/studio/frontend/src/features/chat/stores/chat-runtime-store.ts +++ b/studio/frontend/src/features/chat/stores/chat-runtime-store.ts @@ -3,8 +3,9 @@ import { mirrorHfTokenInto, useHfTokenStore } from "@/features/hub"; import { + type GpuIndexKind, cachedPinnableGpuIndices, - gpuIndexSpaceChangedSinceLastSession, + currentGpuIndexKind, } from "@/hooks/use-gpu-info"; import { toast } from "@/lib/toast"; import { create } from "zustand"; @@ -587,21 +588,24 @@ export function rebalanceSplit( // way to clear it. A null pick (= automatic) passes through unchanged, and an // unpopulated device cache leaves the pick alone (the backend still guards). // -// ``fromPersisted`` gates the index-space-change clear: a pick loaded from a -// saved PerModelConfig may have been stored under the other llama.cpp backend's -// index space (physical CUDA/ROCm ids vs ggml Vulkan ordinals), so drop it when -// the space changed this session. A LIVE store pick (the user just chose it from -// the current picker) is always in the current space, so callers reading -// store.selectedGpuIds must leave fromPersisted false -- else a fresh Vulkan -// selection gets nulled for the whole first post-swap session. +// ``savedKind`` is the GPU index space the pick was made under. Bare ids mean +// different cards across a llama.cpp backend swap (physical CUDA/ROCm ids vs +// ggml Vulkan ordinals), so when the pick's stamped kind no longer matches the +// kind the backend currently reports, the pick is dropped. A pick made in the +// current space (a live picker selection, or a config just stamped on edit) +// carries the current kind and is kept. ``savedKind`` omitted/undefined means +// "unknown, don't apply the index-space check" (callers pass "physical" for a +// pre-stamp stored pick). On a cold cache the current kind is unknown, so the +// check is skipped and a later warm reconcile at the load boundary decides. export function reconcilePersistedGpuIds( ids: number[] | null, - opts: { fromPersisted?: boolean } = {}, + savedKind?: GpuIndexKind | null, ): number[] | null { if (ids == null) return ids; + const current = currentGpuIndexKind(); + if (savedKind != null && current != null && savedKind !== current) return null; const pinnable = cachedPinnableGpuIndices(); if (pinnable === null) return ids; // cache not ready: can't validate, keep it - if (opts.fromPersisted && gpuIndexSpaceChangedSinceLastSession()) return null; const kept = ids.filter((i) => pinnable.includes(i)); return kept.length > 0 ? kept : null; } @@ -631,6 +635,7 @@ export function loadedGpuMemoryFields(resp: { // baseline clears to null so Reset preserves the preference, not a stale mode. return { selectedGpuIds: null, + selectedGpuIdsKind: null, loadedGpuIds: null, loadedGpuMemoryMode: null, gpuLayers: GPU_LAYERS_AUTO, @@ -684,6 +689,9 @@ export function loadedGpuMemoryFields(resp: { moeLayerCount: resp.n_moe_layers ?? null, // The picker reflects what loaded (the request sent the user's pick). selectedGpuIds: gpuIds, + // What the running server loaded is by definition in the current backend's + // index space. + selectedGpuIdsKind: gpuIds == null ? null : currentGpuIndexKind(), loadedGpuIds: gpuIds, ...manualKnobs, }; @@ -909,6 +917,10 @@ type ChatRuntimeStore = { moeLayerCount: number | null; /** Picked physical GPU indices (null = use all / automatic). */ selectedGpuIds: number[] | null; + /** The GPU index space selectedGpuIds is expressed in ("physical" ids vs ggml + * "vulkan" ordinals), stamped when the pick is set so a later reconcile can + * drop it after a llama.cpp backend swap. null when there is no pick. */ + selectedGpuIdsKind: GpuIndexKind | null; loadedGpuIds: number[] | null; /** Persisted: expand every On Device GGUF repo's quantizations by default * instead of waiting for a click. */ @@ -1358,6 +1370,7 @@ export const useChatRuntimeStore = create((set, get) => ({ ggufLayerCount: null, moeLayerCount: null, selectedGpuIds: null, + selectedGpuIdsKind: null, loadedGpuIds: null, expandQuantizations: loadBool(CHAT_EXPAND_QUANTIZATIONS_KEY, false), showAllQuantizations: loadBool(CHAT_SHOW_ALL_QUANTIZATIONS_KEY, true), @@ -1612,6 +1625,7 @@ export const useChatRuntimeStore = create((set, get) => ({ ggufLayerCount: null, moeLayerCount: null, selectedGpuIds: null, + selectedGpuIdsKind: null, loadedGpuIds: null, loadedIsMultimodal: false, loadedIsDiffusion: false, @@ -1915,7 +1929,14 @@ export const useChatRuntimeStore = create((set, get) => ({ setGpuLayers: (gpuLayers) => set({ gpuLayers }), setNCpuMoe: (nCpuMoe) => set({ nCpuMoe }), setSplitRatio: (splitRatio) => set({ splitRatio }), - setSelectedGpuIds: (selectedGpuIds) => set({ selectedGpuIds }), + setSelectedGpuIds: (selectedGpuIds) => + set({ + selectedGpuIds, + // A live picker selection is by definition in the current backend's index + // space; stamp it so a later reconcile keeps it (and drops it only after + // an actual backend swap). null pick carries no space. + selectedGpuIdsKind: selectedGpuIds == null ? null : currentGpuIndexKind(), + }), setExpandQuantizations: (expandQuantizations) => { saveBool(CHAT_EXPAND_QUANTIZATIONS_KEY, expandQuantizations); set({ expandQuantizations }); diff --git a/studio/frontend/src/features/hub/hub-page.tsx b/studio/frontend/src/features/hub/hub-page.tsx index 61bc481410..64d3db9ec3 100644 --- a/studio/frontend/src/features/hub/hub-page.tsx +++ b/studio/frontend/src/features/hub/hub-page.tsx @@ -1189,10 +1189,7 @@ export function ModelsPage() { const previousConfig = currentRuntimePerModelConfig({ includeMaxSeqLength: true, }); - const hasAppliedConfig = applyModelLoadConfigToRuntime(rememberedConfig, { - // rememberedConfig is the resolveInitialConfig storage hit (or null). - fromPersisted: true, - }); + const hasAppliedConfig = applyModelLoadConfigToRuntime(rememberedConfig); void selectModel({ id: runId, ggufVariant: opts.ggufVariant, 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 9993ac0713..2289f40c5b 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 @@ -19,7 +19,7 @@ import { readPersistedSpeculativeType, useChatRuntimeStore, } from "@/features/chat"; -import { useGpuDevices } from "@/hooks/use-gpu-info"; +import { currentGpuIndexKind, useGpuDevices } from "@/hooks/use-gpu-info"; import { ChevronDownStandardIcon } from "@/lib/chevron-icons"; import { toast } from "@/lib/toast"; import { ArrowLeft01Icon } from "@hugeicons/core-free-icons"; @@ -265,7 +265,14 @@ function GpuMemorySettings({ ? current.filter((i) => i !== index) : [...current, index].sort((a, b) => a - b); if (next.length === 0) return; // keep at least one GPU selected - update({ selectedGpuIds: next.length === all.length ? null : next }); + const nextIds = next.length === all.length ? null : next; + update({ + selectedGpuIds: nextIds, + // Stamp the space this fresh edit is in so it is never dropped as a stale + // cross-space restore; clearing to all (null) drops the stamp. + selectedGpuIdsIndexKind: + nextIds == null ? undefined : (currentGpuIndexKind() ?? undefined), + }); }; return ( <> @@ -300,6 +307,7 @@ function GpuMemorySettings({ gpuLayers: undefined, nCpuMoe: undefined, selectedGpuIds: undefined, + selectedGpuIdsIndexKind: undefined, }, ) } diff --git a/studio/frontend/src/features/model-picker/model-config/apply-per-model-config.ts b/studio/frontend/src/features/model-picker/model-config/apply-per-model-config.ts index 71b82c934e..f893e737dc 100644 --- a/studio/frontend/src/features/model-picker/model-config/apply-per-model-config.ts +++ b/studio/frontend/src/features/model-picker/model-config/apply-per-model-config.ts @@ -10,10 +10,7 @@ import { reconcilePersistedGpuIds, useChatRuntimeStore, } from "@/features/chat"; -import { - cachedPinnableGpuIndices, - ensureGpuDeviceCache, -} from "@/hooks/use-gpu-info"; +import type { GpuIndexKind } from "@/hooks/use-gpu-info"; import { DEFAULT_PER_MODEL_CONFIG, type PerModelConfig, @@ -24,18 +21,7 @@ function cleanTemplate(value: string | null | undefined): string | null { return value?.trim() ? value : null; } -export function applyPerModelConfigToRuntime( - config: PerModelConfig, - // ``fromPersisted`` must be true only when ``config`` was actually restored - // from storage (a remembered per-model config). The GPU index-space - // invalidation (physical CUDA/ROCm ids vs ggml Vulkan ordinals, after a - // backend swap) applies only to such picks. A freshly edited config -- Run - // settings, a compare-pane selector, a cancel-restore of the in-session - // config -- is already in the current index space, so it stays live (false) - // and its pick is never cleared. - opts: { fromPersisted?: boolean } = {}, -): void { - const fromPersisted = opts.fromPersisted ?? false; +export function applyPerModelConfigToRuntime(config: PerModelConfig): void { // Fall back to the standing default when the model has no saved // maxSeqLength. maxSeqLength is the only per-model field carried on // params (the rest are reset below), so without this a model with no @@ -47,23 +33,22 @@ export function applyPerModelConfigToRuntime( if (maxSeqLength !== store.params.maxSeqLength) { store.setParams({ ...store.params, maxSeqLength }); } - // reconcilePersistedGpuIds can only decide the index-space question once the - // GPU cache is warm. This runs synchronously on model selection, which can - // happen before any GPU hook has fetched /api/system. For a persisted pick on - // a cold cache we cannot yet tell whether the saved ids belong to the current - // backend's index space, so parking the raw ids in the store would let the - // load path snapshot and send them as-is (pinning the wrong card after a - // swap). Park null instead (all GPUs -- safe) and fill in the validated pick - // once the cache warms. Live picks are current-space, so they pass through. - const coldCache = cachedPinnableGpuIndices() === null; - const parkColdPersistedPick = - fromPersisted && coldCache && config.selectedGpuIds != null; + // The pick's index space is intrinsic to the config: its + // selectedGpuIdsIndexKind stamp (a fresh edit stamps the current kind; a + // stored pick carries the kind it was saved under; absent = a pre-stamp + // physical pick). reconcilePersistedGpuIds drops it only when that stamp no + // longer matches the current backend, so a same-backend restore keeps the + // pick and a cross-space one is cleared -- no call-site "is this persisted?" + // guessing. On a cold cache the reconcile keeps the ids; the stamp is carried + // into the store so the load-boundary reconcile of the live value can decide. + const savedKind: GpuIndexKind | null = + config.selectedGpuIds == null + ? null + : (config.selectedGpuIdsIndexKind ?? "physical"); const reconciledGpuIds = config.selectedGpuIds === undefined ? null - : parkColdPersistedPick - ? null - : reconcilePersistedGpuIds(config.selectedGpuIds, { fromPersisted }); + : reconcilePersistedGpuIds(config.selectedGpuIds, savedKind); useChatRuntimeStore.setState({ customContextLength: config.customContextLength ?? null, kvCacheDtype: config.kvCacheDtype ?? null, @@ -83,33 +68,15 @@ export function applyPerModelConfigToRuntime( nCpuMoe: config.nCpuMoe ?? 0, splitRatio: null, selectedGpuIds: reconciledGpuIds, + selectedGpuIdsKind: reconciledGpuIds == null ? null : savedKind, }); - - if (parkColdPersistedPick) { - // parkColdPersistedPick already established selectedGpuIds != null. - const persistedIds = config.selectedGpuIds as number[]; - void ensureGpuDeviceCache().then(() => { - // Only fill in the parked pick if the user has not chosen one meanwhile - // (reconciledGpuIds is null here, so the store must still read null). - if (useChatRuntimeStore.getState().selectedGpuIds !== reconciledGpuIds) { - return; - } - const revalidated = reconcilePersistedGpuIds(persistedIds, { - fromPersisted: true, - }); - if (revalidated !== reconciledGpuIds) { - useChatRuntimeStore.setState({ selectedGpuIds: revalidated }); - } - }); - } } export function applyModelLoadConfigToRuntime( config: PerModelConfig | null | undefined, - opts: { fromPersisted?: boolean } = {}, ): boolean { const hasConfig = config != null; - applyPerModelConfigToRuntime(config ?? DEFAULT_PER_MODEL_CONFIG, opts); + applyPerModelConfigToRuntime(config ?? DEFAULT_PER_MODEL_CONFIG); return hasConfig; } @@ -134,6 +101,10 @@ export function currentRuntimePerModelConfig( gpuLayers: s.gpuLayers, nCpuMoe: s.nCpuMoe, selectedGpuIds: s.selectedGpuIds, + // Carry the index space the live pick is in so a save/restore round-trip + // (and a cancel-restore of this snapshot) can drop it after a backend swap. + selectedGpuIdsIndexKind: + s.selectedGpuIds == null ? undefined : (s.selectedGpuIdsKind ?? undefined), }; } 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 0b03423736..91f28d95b4 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 @@ -25,6 +25,13 @@ export interface PerModelConfig { gpuLayers?: number; nCpuMoe?: number; selectedGpuIds?: number[] | null; + // The GPU index space selectedGpuIds was saved under ("physical" CUDA/ROCm + // ids vs ggml "vulkan" ordinals). Bare ids mean different cards across a + // llama.cpp backend swap, so this stamp lets reconcilePersistedGpuIds drop a + // pick whose space no longer matches. Absent on pre-stamp blobs and on picks + // with no selection -- treated as "physical", the only space that existed + // before Vulkan support. + selectedGpuIdsIndexKind?: "vulkan" | "physical"; } export const DEFAULT_PER_MODEL_CONFIG: PerModelConfig = { @@ -88,6 +95,7 @@ const STORED_CONFIG_FIELDS = new Set([ "gpuLayers", "nCpuMoe", "selectedGpuIds", + "selectedGpuIdsIndexKind", ]); function normalizeGpuFields(partial: RawConfig): { @@ -95,12 +103,14 @@ function normalizeGpuFields(partial: RawConfig): { gpuLayers?: number; nCpuMoe?: number; selectedGpuIds?: number[] | null; + selectedGpuIdsIndexKind?: "vulkan" | "physical"; } { const out: { gpuMemoryMode?: "auto" | "manual"; gpuLayers?: number; nCpuMoe?: number; selectedGpuIds?: number[] | null; + selectedGpuIdsIndexKind?: "vulkan" | "physical"; } = {}; // Only "manual" is a real override; persisting "auto" would pin the model and // stop it following later changes to the global GPU Memory preference. @@ -130,6 +140,15 @@ function normalizeGpuFields(partial: RawConfig): { ) { out.selectedGpuIds = partial.selectedGpuIds.map((n) => Math.trunc(n)); } + // Only meaningful alongside a real selection; a null/absent pick carries no + // space, so drop the stamp to keep the stored blob minimal. + if ( + Array.isArray(out.selectedGpuIds) && + (partial.selectedGpuIdsIndexKind === "vulkan" || + partial.selectedGpuIdsIndexKind === "physical") + ) { + out.selectedGpuIdsIndexKind = partial.selectedGpuIdsIndexKind; + } return out; } diff --git a/studio/frontend/src/hooks/use-gpu-info.ts b/studio/frontend/src/hooks/use-gpu-info.ts index 2a9fcb43cd..a759cf0081 100644 --- a/studio/frontend/src/hooks/use-gpu-info.ts +++ b/studio/frontend/src/hooks/use-gpu-info.ts @@ -54,29 +54,24 @@ let systemPromise: Promise | null = null; // within the index space they were saved in: physical CUDA/ROCm ids, or ggml // Vulkan ordinals on a Vulkan build. Swapping the llama.cpp backend flips that // space while keeping many numbers valid (Vulkan ordinal 1 exists but may be a -// different card than physical id 1), so remember the last-seen space and flag -// a change; reconcilePersistedGpuIds clears saved picks for the session. -const GPU_INDEX_KIND_STORAGE_KEY = "unsloth-gpu-pick-index-kind"; -let gpuIndexSpaceChanged = false; +// different card than physical id 1). Each saved pick is stamped with the kind +// it was made under (PerModelConfig.selectedGpuIdsIndexKind, and the runtime +// store's selectedGpuIdsKind); reconcilePersistedGpuIds drops a pick whose stamp +// no longer matches the kind the backend reports here. +export type GpuIndexKind = "vulkan" | "physical"; -function noteGpuIndexKind(data: SystemInfoResponse | null): void { - if (!data) return; - const kind = (data.gpu?.gguf_devices ?? []).length ? "vulkan" : "physical"; - try { - const last = window.localStorage.getItem(GPU_INDEX_KIND_STORAGE_KEY); - // Installs predating the marker could only save physical picks, so a - // missing marker on a Vulkan host counts as a space change too. - gpuIndexSpaceChanged = last === null ? kind === "vulkan" : last !== kind; - window.localStorage.setItem(GPU_INDEX_KIND_STORAGE_KEY, kind); - } catch { - gpuIndexSpaceChanged = false; - } +function systemGpuIndexKind( + data: SystemInfoResponse | null, +): GpuIndexKind | null { + if (!data) return null; + return (data.gpu?.gguf_devices ?? []).length ? "vulkan" : "physical"; } -/** True when the GPU index space differs from the one the last session's picks - * were saved in; stays true for the whole session so late restores clear too. */ -export function gpuIndexSpaceChangedSinceLastSession(): boolean { - return gpuIndexSpaceChanged; +/** The index space the backend currently reports gpu_ids in, or null when the + * /api/system cache has not populated yet (so callers cannot decide and must + * defer the index-space judgement to a later warm reconcile). */ +export function currentGpuIndexKind(): GpuIndexKind | null { + return systemGpuIndexKind(cachedSystem); } async function fetchSystemOnce(): Promise { @@ -87,7 +82,6 @@ async function fetchSystemOnce(): Promise { const res = await authFetch("/api/system"); if (!res.ok) throw new Error(`HTTP ${res.status}`); cachedSystem = (await res.json()) as SystemInfoResponse; - noteGpuIndexKind(cachedSystem); return cachedSystem; } catch { systemPromise = null; // reset so a later call retries (backend not ready) diff --git a/tests/studio/test_model_picker_contracts.py b/tests/studio/test_model_picker_contracts.py index 45d2947dcd..ca115620b8 100644 --- a/tests/studio/test_model_picker_contracts.py +++ b/tests/studio/test_model_picker_contracts.py @@ -157,11 +157,12 @@ def test_compare_load_uses_each_models_gpu_config(): assert "ownConfig.gpuLayers ?? compareLoadKnobs.gpuLayers" in src assert "ownConfig.nCpuMoe ?? compareLoadKnobs.nCpuMoe" in src assert "if (ownConfig.selectedGpuIds != null)" in src - # A compare pane reconciles its own config's GPU pick, but the index-space - # invalidation (fromPersisted) must apply only to a real storage restore -- - # never to an explicit sel.config, which is a fresh current-space pane pick. - assert "reconcilePersistedGpuIds(ownConfig.selectedGpuIds, {" in src - assert "fromPersisted: !config && ownRemembered" in src + # A compare pane reconciles its own config's GPU pick against the index space + # stamped on that pick, so a fresh sel.config (current-kind) stays live while + # a cross-space storage restore is dropped -- no call-site "is this a real + # restore?" heuristic. Unstamped (pre-Vulkan) picks default to physical. + assert "reconcilePersistedGpuIds(" in src + assert 'ownConfig.selectedGpuIdsIndexKind ?? "physical"' in src for field in ( "gpu_memory_mode: effectiveGpuMemoryMode", "gpu_layers: effectiveGpuLayers", @@ -171,22 +172,37 @@ def test_compare_load_uses_each_models_gpu_config(): assert field in src -def test_gpu_index_space_invalidation_only_for_storage_restores(): +def test_gpu_pick_carries_index_kind_stamp(): """The GPU index-space invalidation (physical CUDA/ROCm ids vs ggml Vulkan - ordinals after a backend swap) must apply only to picks actually restored - from storage. A freshly edited config -- an explicit selection.config, a - compare-pane sel.config -- is already current-space and must stay live, or - selecting a Vulkan ordinal and loading clears it and loads on all GPUs.""" - # chat-page load-on-selection: fromPersisted only for the rememberedConfigFor - # fallback, never for an explicit selection.config. - chat = _read("features/chat/chat-page.tsx") - assert "fromPersisted: !selection.config" in chat + ordinals after a backend swap) is driven by a kind stamp carried WITH each + pick, not a call-site persisted/live guess. A pick made in the current space + (a live picker selection or a fresh Run-settings edit) is stamped with the + current kind and kept; a stored pick from the other space is dropped. This + is what keeps a fresh Vulkan pick from being cleared while still dropping a + stale physical pin, across the store, the compare pane, and Run settings.""" + # The live picker stamps the current kind onto the store companion field. + store = _read("features/chat/stores/chat-runtime-store.ts") + assert "selectedGpuIdsKind: GpuIndexKind | null" in store + assert ( + "selectedGpuIdsKind: selectedGpuIds == null ? null : currentGpuIndexKind()" + in store + ) + # reconcile drops a pick only when its stamped kind differs from the current. + assert "savedKind != null && current != null && savedKind !== current" in store - # apply-per-model-config: defaults to live, and on a cold cache it parks a - # persisted pick at null rather than leaking the raw ids to the load path. + # apply-per-model-config reads the config's stamp (default physical) and + # carries it into the store; no fromPersisted / cold-cache parking anymore. apply = _read("features/model-picker/model-config/apply-per-model-config.ts") - assert "opts: { fromPersisted?: boolean } = {}" in apply - assert "const parkColdPersistedPick" in apply + assert 'config.selectedGpuIdsIndexKind ?? "physical"' in apply + assert "selectedGpuIdsKind: reconciledGpuIds == null ? null : savedKind" in apply + assert "fromPersisted" not in apply + assert "parkColdPersistedPick" not in apply + + # Run settings stamps the current kind when the user edits the GPU picker, + # so an edited pick is never mistaken for a stale cross-space restore. + page = _read("features/model-picker/components/model-config-page.tsx") + assert "currentGpuIndexKind()" in page + assert "selectedGpuIdsIndexKind:" in page def test_active_native_gguf_metadata_uses_path_token():