Key the Hub's per-model settings by the repo id, not by the load path

A repo cached outside the active HF cache reports load_id as its snapshot path
(cache_inventory), which is what the loader needs, but the chat picker and the
auto-switch index both name that repo by repo_id. The new Hub settings page was
saving under the load id, so the settings landed on a key no other load reads:
the picker, an auto-load and an OpenAI-compatible request all fell back to
defaults, and a server override already stored under the repo id could win
against the save.

ModelPickTarget now carries configId for the case where the storage identity is
not the loadable one. Every read, write and server mirror in ModelConfigPage
uses it; the chat template and GGUF header probes keep target.id, since they
have to open the model. The Hub sets it for cache rows and resolves its own
load through the same helper, so a config saved from the settings page is the
one a later load finds. Rows whose load id is already their identity, which is
every local row and every repo in the active cache, are unaffected.

Verified against the real per-model-config module under node: saved under the
snapshot path, a picker read reports remembered=false and the default max
sequence length; saved under the repo id it reports remembered=true and 8192.
This commit is contained in:
danielhanchen 2026-07-28 17:07:11 +00:00
commit 1796ebf330
4 changed files with 82 additions and 7 deletions

View file

@ -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,

View file

@ -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,
);

View file

@ -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;
}

View file

@ -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"