Clear the derived gguf key on forget and key cached repos by repo id

Three fixes.

A standalone .gguf is keyed by its bare path, but a load also reads the
`<path>:LABEL` entry derived from the filename, which is how the picker
keyed the same file before and what the backfill carries over from an
upgraded browser. `resolve_model_override_key` cannot fold a bare path
onto that spelling, so forgetting cleared a key that was never there and
left the real one applying to every later API load, with the settings
gone from the UI and nothing left to reach them. The remove branch now
clears the derived key too, only for a bare `.gguf` path and only
through the resolver, so an ambiguous fold removes nothing.

`modelConfigIdentity` decided from the view kind what its own comment
says depends on the row. Discover resolves a repo in an inactive HF
cache into a discover-kind view whose resource is still the cache row
with its snapshot-path run id, so Run from Discover read a key the
Downloaded row never writes and loaded with default context, GPU and
template, silently. Keyed off the resource instead.

The detail view's settings button took the card's quant as given. That
quant is a choice only when the user picked it in the selector;
otherwise it comes off a store nothing re-reads while the window keeps
focus. The card now says which it is, and a derived one defers to a
fresh status read.
This commit is contained in:
danielhanchen 2026-07-29 07:38:23 +00:00
commit 8ffcba546c
5 changed files with 190 additions and 19 deletions

View file

@ -399,6 +399,35 @@ def _bare_model_id(model_id: str) -> Optional[str]:
return split[0] if split is not None else None
def _legacy_standalone_gguf_key(model_id: str) -> Optional[str]:
"""The stored ``<path>:LABEL`` entry for a bare standalone .gguf path, if any.
A loose file has no quant to choose between, so it is keyed by the bare path,
but the label derived from its filename is never empty and that is how the
picker keyed the same file before, so an upgraded install carries entries
under it. The auto-switch loader reads that spelling after the bare path
misses; resolve_model_override_key does not, since folding a POSIX path only
touches an existing suffix. None for an id that already names a quant, for a
repo id, and when nothing is stored under the derived key.
"""
import os
if not model_id.lower().endswith(".gguf"):
return None
# Already qualified, so the caller named the entry it meant. Mirrors the
# loader, which derives a label only when the resolver gave it no variant.
if _bare_model_id(model_id) is not None:
return None
from hub.utils.gguf import extract_quant_label
label = extract_quant_label(os.path.basename(model_id))
if not label:
return None
# Through the resolver rather than a raw lookup: the browser lowercases the
# variant, and an ambiguous fold resolves to nothing rather than guessing.
return resolve_model_override_key(f"{model_id}:{label}")
@router.put("/openai-auto-switch/overrides", response_model = ModelOverridesResponse)
def update_openai_auto_switch_override(
payload: ModelOverridePayload, current_subject: str = Depends(get_current_subject)
@ -450,6 +479,16 @@ def update_openai_auto_switch_override(
# casing before storing, so 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)
# A standalone .gguf is keyed by its bare path now, but a load also
# reads the filename-derived <path>:LABEL entry an upgraded install
# still holds. Clearing only what the resolver sees leaves that one
# applying to every later API load, with the settings gone from the
# UI and no way left to reach them.
legacy_id = _legacy_standalone_gguf_key(payload.model_id)
if legacy_id and legacy_id != target_id:
set_model_override(
legacy_id, llama_extra_args = [], max_seq_length = None,
)
else:
# Save under the key a load resolves to, as the removal branch does.
# Saving the literal id leaves two keys for one model, which makes every

View file

@ -5016,6 +5016,62 @@ def test_removal_of_a_path_still_only_touches_the_exact_key(monkeypatch):
assert settings.get_model_override("/models/foo.gguf")["max_seq_length"] == 8192
def test_forget_clears_the_filename_derived_key_a_load_still_reads(monkeypatch):
# The picker keyed a standalone .gguf by the quant label from its filename
# before it settled on the bare path, and the backfill carries those entries
# over from an upgraded browser. Forget sends the bare path, which the
# resolver cannot fold onto the suffixed key, while the loader reads that
# suffixed key after the bare one misses: the settings leave the UI and go
# on being applied to every API load, unreachable.
import routes.settings as settings_route
_mock_override_store(monkeypatch)
settings.set_model_override(
"/models/Qwen3-8B-Q4_K_M.gguf:q4_k_m", max_seq_length = 8192,
)
settings_route.update_openai_auto_switch_override(
settings_route.ModelOverridePayload(
model_id = "/models/Qwen3-8B-Q4_K_M.gguf", remove = True,
),
"tester",
)
assert settings.get_model_override("/models/Qwen3-8B-Q4_K_M.gguf:Q4_K_M") == {}
assert settings.get_model_overrides() == {}
def test_forget_leaves_another_file_own_derived_key_alone(monkeypatch):
# The derived key is built from the forgotten file's own path and its own
# label, so a neighbour that happens to share a quant keeps its settings.
import routes.settings as settings_route
_mock_override_store(monkeypatch)
settings.set_model_override("/models/Other-Q4_K_M.gguf:q4_k_m", max_seq_length = 4096)
settings_route.update_openai_auto_switch_override(
settings_route.ModelOverridePayload(
model_id = "/models/Qwen3-8B-Q4_K_M.gguf", remove = True,
),
"tester",
)
assert settings.get_model_override("/models/Other-Q4_K_M.gguf:Q4_K_M")[
"max_seq_length"
] == 4096
def test_forget_of_a_repo_quant_key_derives_nothing(monkeypatch):
# Only a bare .gguf path derives a label. An id that already names a quant is
# the entry the caller meant, and a repo id never gets a filename read out of
# it, so neither reaches the extra removal.
import routes.settings as settings_route
_mock_override_store(monkeypatch)
settings.set_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", remove = True),
"tester",
)
assert settings.get_model_override("unsloth/b-gguf:q4_k_m")["max_seq_length"] == 8192
def test_a_tag_that_names_no_quant_resolves_to_the_repo(monkeypatch):
# A downloaded but unloaded GGUF asked for as org/model:latest missed the resolver,
# so the switch could not load it (404ing on a quant that was never a quant with

View file

@ -105,8 +105,14 @@ interface LocalOnDeviceCardProps {
onEject?: () => void;
onTrain?: () => void;
onChange?: () => void;
/** Open settings for the quant this card is showing. */
onOpenSettings?: (ggufVariant: string | null) => void;
/**
* Open settings for the quant this card is showing.
*
* ``quantIsUserPicked`` says whether that quant came from a pick in this
* card's selector or was derived from the resident model, which decides
* whether a fresher status read may override it.
*/
onOpenSettings?: (ggufVariant: string | null, quantIsUserPicked: boolean) => void;
}
function formatAdapterLabel(
@ -371,6 +377,14 @@ export function LocalOnDeviceCard({
)?.quant ??
sortedVariants?.[0]?.quant ??
null);
// Only the first branch above is a choice; the rest are fallbacks that read the
// store, and the store can be stale for as long as this window keeps focus.
const quantIsUserPicked = Boolean(
selectedVariantOverride &&
sortedVariants?.some((variant) =>
ggufVariantsMatch(variant.quant, selectedVariantOverride),
),
);
const selectedVariant =
sortedVariants?.find((variant) =>
ggufVariantsMatch(variant.quant, selectedQuant),
@ -587,7 +601,9 @@ export function LocalOnDeviceCard({
label={`Settings for ${repoId}`}
// Pass the quant this card resolved, so the settings page edits the
// variant on screen rather than the repo.
onClick={() => onOpenSettings(selectedQuant ?? null)}
onClick={() =>
onOpenSettings(selectedQuant ?? null, quantIsUserPicked)
}
/>
)}
{canUpdate && (

View file

@ -125,11 +125,19 @@ import type {
// handed: a repo cached outside the active HF cache loads by snapshot path, while
// the chat picker and the auto-switch index key it by repo id, so saving under the
// path strands the settings. Local rows are keyed by load id in both places.
// The row decides that, not the view it is being shown in: the same cached repo
// is reachable from Discover, where the kind is "discover" while the resource is
// still the cache row with its snapshot-path run id. Keying that off the kind
// stranded the settings for exactly the case this exists to handle. `hub_cache`
// is set only for a cached repo; a local row in the active HF cache is `hf_cache`
// and stays on its run id, as the Downloaded row keys it.
function modelConfigIdentity(
kind: SelectedModelView["kind"],
resource: SelectedResourceRef,
): string {
if (kind !== "cache") return resource.runId;
if (kind !== "cache" && resource.source !== "hub_cache") {
return resource.runId;
}
return resource.repoId ?? resource.runId;
}
@ -1469,25 +1477,49 @@ export function ModelsPage() {
// Opened from the detail view's on-device card, which passes in the quant it
// resolved rather than making this re-derive it.
const openSelectedModelSettings = useCallback(
(ggufVariant: string | null) => {
async (ggufVariant: string | null, quantIsUserPicked = false) => {
if (!selectedModel) return;
// Share the sequence with openModelSettings: a pending variant lookup for
// another row must not land on top of this one.
const openSeq = ++settingsOpenSeq.current;
let variant = ggufVariant;
// The card derived this quant from the store's active variant, and nothing
// re-reads status while this window keeps focus, so an API-driven switch
// since the last focus event leaves it naming the model that switch
// displaced. A quant the user chose in the card is theirs and stands; a
// derived one defers to whatever a fresh read says is actually loaded.
if (!quantIsUserPicked) {
await refreshResidentModelStatus();
if (settingsOpenSeq.current !== openSeq) return;
const settled = useChatRuntimeStore.getState();
const settledCheckpoint =
settled.params.checkpoint &&
!isExternalModelId(settled.params.checkpoint)
? settled.params.checkpoint
: null;
// Every name this model answers to, as the row menu path matches them.
const aliases = [
selectedModel.resource.runId,
selectedModel.resource.repoId,
selectedModel.resource.localPath,
];
if (
settled.activeGgufVariant &&
aliases.some((alias) => modelIdsMatch(alias, settledCheckpoint))
) {
variant = settled.activeGgufVariant;
}
}
// The card passes null while its variant lookup is pending or after it failed,
// so this needs the same guard openModelSettings applies: a model that needs a
// quant cannot be configured without one.
if (
!ggufVariant &&
selectedModel.isGguf &&
selectedModel.requiresVariant
) {
if (!variant && selectedModel.isGguf && selectedModel.requiresVariant) {
toast.error("Couldn't determine which quant to configure.", {
description:
"Settings for this model are per quant. Check the connection or the model's cache, then try again.",
});
return;
}
// Share the sequence with openModelSettings: a pending variant lookup for
// another row must not land on top of this one.
settingsOpenSeq.current += 1;
const id = selectedModel.resource.runId;
const configId = modelConfigIdentity(
selectedModel.kind,
@ -1497,8 +1529,8 @@ export function ModelsPage() {
setSettingsTarget({
id,
configId,
displayName: ggufVariant ? `${leaf} · ${ggufVariant}` : leaf,
ggufVariant,
displayName: variant ? `${leaf} · ${variant}` : leaf,
ggufVariant: variant,
isGguf: selectedModel.isGguf,
apiLoadable:
selectedModel.isGguf &&
@ -1506,14 +1538,14 @@ export function ModelsPage() {
meta: {
source: "local",
isLora: selectedModel.modelFormat === "adapter",
ggufVariant: ggufVariant ?? undefined,
ggufVariant: variant ?? undefined,
isGguf: selectedModel.isGguf,
isDownloaded: selectedModel.isDownloaded,
contextLength: null,
},
});
},
[selectedModel],
[selectedModel, refreshResidentModelStatus],
);
// Whether the settings page is open on the model that is actually loaded, so it
// can show the live launch config. A GGUF loaded from an inactive HF cache or

View file

@ -1150,7 +1150,9 @@ def test_detail_settings_need_a_resolved_quant():
bare-key fallback would apply it. openModelSettings already refuses; this
entry point has to refuse the same way."""
src = " ".join(_read("features/hub/hub-page.tsx").split())
guard = "if ( !ggufVariant && selectedModel.isGguf && selectedModel.requiresVariant )"
# `variant`, not the argument: a derived quant may have been replaced by the
# resident one first, and the guard has to judge what will actually be saved.
guard = "if (!variant && selectedModel.isGguf && selectedModel.requiresVariant) {"
assert guard in src
assert src.count("Couldn't determine which quant to configure.") == 2
@ -1204,7 +1206,7 @@ def test_cached_repo_settings_are_keyed_by_the_repo_id():
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 'if (kind !== "cache" && resource.source !== "hub_cache") {' 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
@ -1555,3 +1557,29 @@ def test_settings_open_reads_status_before_resolving_the_quant():
assert "const refreshResidentModelStatus = useCallback((): Promise<void> => {" in page
assert "const [res] = await Promise.all([ listGgufVariants(" in page
assert "refreshResidentModelStatus(), ]);" in page
def test_cached_repo_settings_key_follows_the_row_not_the_view():
"""A repo in an inactive HF cache loads by snapshot path while its settings
are keyed by repo id. The same row is reachable from Discover, where the view
kind is "discover" and only the resource says it is a cache row, so keying
off the kind stranded the settings for exactly the case the helper exists to
handle: Run from Discover looked up the snapshot path, found nothing, and
loaded with default context, GPU and template."""
page = " ".join(_read("features/hub/hub-page.tsx").split())
assert (
'if (kind !== "cache" && resource.source !== "hub_cache") {' in page
)
def test_detail_settings_defers_a_derived_quant_to_a_fresh_status_read():
"""The on-device card resolves the quant it shows from the store's active
variant, and nothing re-reads status while the window keeps focus, so an
API-driven switch leaves that quant naming the model it displaced. A quant
the user picked in the card's selector is a choice and must survive; only a
derived one defers to the read."""
page = " ".join(_read("features/hub/hub-page.tsx").split())
card = " ".join(_read("features/hub/catalog/local-on-device-card.tsx").split())
assert "if (!quantIsUserPicked) { await refreshResidentModelStatus();" in page
assert "variant = settled.activeGgufVariant;" in page
assert "onOpenSettings(selectedQuant ?? null, quantIsUserPicked)" in card