diff --git a/studio/frontend/src/features/hub/hub-page.tsx b/studio/frontend/src/features/hub/hub-page.tsx index 714a3023ed..ae152fbf3a 100644 --- a/studio/frontend/src/features/hub/hub-page.tsx +++ b/studio/frontend/src/features/hub/hub-page.tsx @@ -109,8 +109,23 @@ import type { ModelsTab, ResourceTypeFilter, SelectedModelView, + SelectedResourceRef, } from "./types"; +// What per-model settings are keyed by, which is not always what the loader is +// handed: a repo cached outside the active HF cache loads by snapshot path, +// while the chat picker (toCachedModelRepo) and the auto-switch index both key +// it by repo id. Saving under the path would leave the settings where no other +// load looks for them. Local rows are keyed by their load id in both places, so +// they keep it. +function modelConfigIdentity( + kind: SelectedModelView["kind"], + resource: SelectedResourceRef, +): string { + if (kind !== "cache") return resource.runId; + return resource.repoId ?? resource.runId; +} + const MODELS_TAB_STORAGE_KEY = "unsloth.hub.modelsTab"; const ALL_MODELS_VIEW_STORAGE_KEY = "unsloth.hub.allModelsView"; const INVENTORY_SORT_STORAGE_KEY = "unsloth.hub.inventorySort"; @@ -1192,7 +1207,10 @@ export function ModelsPage() { (opts: ModelLoadOptions, isDownloaded: boolean) => { if (!selectedModel) return; const runId = selectedModel.resource.runId; - const resolvedConfig = resolveInitialConfig(runId, opts.ggufVariant); + const resolvedConfig = resolveInitialConfig( + modelConfigIdentity(selectedModel.kind, selectedModel.resource), + opts.ggufVariant, + ); const rememberedConfig = resolvedConfig.remembered ? resolvedConfig.config : null; @@ -1304,9 +1322,13 @@ export function ModelsPage() { if (settingsOpenSeq.current !== openSeq) { return; } - const leaf = id.split(/[\\/]/).filter(Boolean).pop() ?? id; + // A repo in a previous cache loads by snapshot path, so `id` ends in the + // revision hash; name the row by what the user calls it. + const configId = row.kind === "cache" ? row.repoId : id; + const leaf = configId.split(/[\\/]/).filter(Boolean).pop() ?? configId; setSettingsTarget({ id, + configId, displayName: ggufVariant ? `${leaf} · ${ggufVariant}` : leaf, ggufVariant, isGguf: row.isGguf, @@ -1389,9 +1411,14 @@ export function ModelsPage() { // still be pending, and it must not land on top of this one. settingsOpenSeq.current += 1; const id = selectedModel.resource.runId; - const leaf = id.split(/[\\/]/).filter(Boolean).pop() ?? id; + const configId = modelConfigIdentity( + selectedModel.kind, + selectedModel.resource, + ); + const leaf = configId.split(/[\\/]/).filter(Boolean).pop() ?? configId; setSettingsTarget({ id, + configId, displayName: ggufVariant ? `${leaf} · ${ggufVariant}` : leaf, ggufVariant, isGguf: selectedModel.isGguf, 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 adf97553df..635164875b 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 @@ -611,8 +611,12 @@ export function ModelConfigPage({ const loadedMaxContextLength = useChatRuntimeStore( (s) => s.ggufMaxContextLength, ); + // What the settings are stored under, which is not always what loads: see + // ModelPickTarget.configId. Every read, write and mirror below uses it; the + // probes keep target.id, since they have to open the model. + const configId = target.configId ?? target.id; const resolveInitial = () => { - const resolved = resolveInitialConfig(target.id, target.ggufVariant); + const resolved = resolveInitialConfig(configId, target.ggufVariant); if (loadedConfig) { return { config: loadedConfig, remembered: resolved.remembered }; } @@ -873,13 +877,13 @@ export function ModelConfigPage({ const evicted: { modelId: string; ggufVariant: string | null }[] = []; if (remember) { saveFailed = !savePerModelConfig( - target.id, + configId, target.ggufVariant, effectiveRuntimeConfig, evicted, ); } else { - saveFailed = !deletePerModelConfig(target.id, target.ggufVariant); + saveFailed = !deletePerModelConfig(configId, target.ggufVariant); } // Mirror to the server so an OpenAI-compatible API request that loads this // model gets these exact settings, not app defaults. Best-effort and @@ -895,7 +899,7 @@ export function ModelConfigPage({ // list that no API request can ever apply. if (!saveFailed && (target.apiLoadable ?? target.isGguf)) { syncModelOverride( - target.id, + configId, target.ggufVariant, remember ? effectiveRuntimeConfig : null, ); diff --git a/studio/frontend/src/features/model-picker/components/model-selector/types.ts b/studio/frontend/src/features/model-picker/components/model-selector/types.ts index e92a7f3e57..980d9ed30e 100644 --- a/studio/frontend/src/features/model-picker/components/model-selector/types.ts +++ b/studio/frontend/src/features/model-picker/components/model-selector/types.ts @@ -58,6 +58,16 @@ export interface ModelPickTarget { * Defaults to isGguf where a caller does not know. */ apiLoadable?: boolean; + /** + * Identity the saved settings are keyed by, when that is not what loads. + * + * A repo cached outside the active HF cache loads by snapshot path, while the + * picker and the auto-switch index keep its settings under the repo id. + * Keying the save by the path would strand the settings where no load looks + * for them. Probes that need something openable (the chat template, the GGUF + * header) keep using `id`. Defaults to `id`. + */ + configId?: string; meta: ModelSelectorChangeMeta; } diff --git a/tests/studio/test_model_picker_contracts.py b/tests/studio/test_model_picker_contracts.py index 0387ab7462..b8b7fbb213 100644 --- a/tests/studio/test_model_picker_contracts.py +++ b/tests/studio/test_model_picker_contracts.py @@ -840,3 +840,37 @@ def test_ollama_models_are_not_advertised_as_api_loadable(): assert ( "Ollama's\n scanner is skipped" in backend or "scanner is skipped" in backend ), "the rule this mirrors" + + +def test_cached_repo_settings_are_keyed_by_the_repo_id(): + """A repo cached outside the active HF cache reports load_id = the snapshot + path (hub/services/cache_inventory.py), while the chat picker and the + auto-switch index key it by repo_id. Keying the Hub's settings by the load id + saved them where no other load looks, so they silently never applied.""" + config_page = " ".join( + _read("features/model-picker/components/model-config-page.tsx").split() + ) + assert "const configId = target.configId ?? target.id;" in config_page + for call in ( + "resolveInitialConfig(configId, target.ggufVariant)", + "savePerModelConfig( configId, target.ggufVariant,", + "deletePerModelConfig(configId, target.ggufVariant)", + "syncModelOverride( configId, target.ggufVariant,", + ): + assert call in config_page, call + # The probes have to open the model, so they keep the load id. + assert "useDefaultChatTemplate( target.id," in config_page + assert "model_path: target.id," in config_page + + hub = " ".join(_read("features/hub/hub-page.tsx").split()) + assert 'if (kind !== "cache") return resource.runId;' in hub + assert "return resource.repoId ?? resource.runId;" in hub + # Both openers and the Hub's own load resolve through it. + assert hub.count("modelConfigIdentity(") == 3 + assert 'const configId = row.kind === "cache" ? row.repoId : id;' in hub + assert hub.count("configId,") >= 2 + + backend = ( + WORKDIR / "studio" / "backend" / "hub" / "tests" / "test_model_services.py" + ).read_text(encoding = "utf-8") + assert 'fields["load_id"] == str(snapshot)' in backend, "the rule this mirrors"