studio: clear server overrides for models evicted from local storage
Saving a config can push the browser's map over its entry or byte budget, at which point older models are silently dropped and the save still reports success. Their server-side overrides survived, so API loads kept applying settings that nothing in the UI showed any more and nothing could forget. savePerModelConfig now reports what it evicted, and the caller clears those server entries alongside the one it saved. Removal also had to resolve the way lookup does. Storage keys are normalized, so an evicted entry comes back lowercased while the server may hold the repo's real casing; deleting only the literal key would leave the entry a load still resolves to. Read and remove now share resolve_model_override_key, so what a load applies and what forgetting clears cannot disagree. Path ids still match exactly, so one file is never forgotten by clearing its case-variant neighbour.
This commit is contained in:
parent
9c722b9059
commit
baf11a01f7
5 changed files with 90 additions and 12 deletions
|
|
@ -43,6 +43,7 @@ from utils.openai_auto_switch_settings import (
|
|||
get_auto_unload_keep_kv,
|
||||
get_model_overrides,
|
||||
get_openai_auto_switch_enabled,
|
||||
resolve_model_override_key,
|
||||
get_stored_auto_unload_idle_seconds,
|
||||
set_model_override,
|
||||
set_openai_auto_switch,
|
||||
|
|
@ -394,7 +395,11 @@ def update_openai_auto_switch_override(
|
|||
# form field must not turn "forget this model" into an update that
|
||||
# keeps it. Only the explicit flag short-circuits; the legacy
|
||||
# inferred path still just gates launch-flag carry-over.
|
||||
set_model_override(payload.model_id, llama_extra_args = [], max_seq_length = None)
|
||||
# Remove the key a load would actually resolve to, not just the
|
||||
# literal one sent: the browser normalizes casing before storing, so
|
||||
# the two can differ and 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)
|
||||
else:
|
||||
set_model_override(
|
||||
payload.model_id,
|
||||
|
|
|
|||
|
|
@ -4404,3 +4404,34 @@ def test_a_non_gpu_load_failure_is_not_retried(monkeypatch):
|
|||
with pytest.raises(HTTPException):
|
||||
_run_hook("unsloth/B-GGUF")
|
||||
assert calls["n"] == 1
|
||||
|
||||
|
||||
def test_removal_clears_the_entry_a_load_would_actually_resolve(monkeypatch):
|
||||
# The browser normalizes casing before storing, so a forget request can carry
|
||||
# a different casing than the stored key. Removing only the literal key would
|
||||
# leave the entry a load still resolves to, with no UI able to clear it.
|
||||
import routes.settings as settings_route
|
||||
|
||||
_mock_override_store(monkeypatch)
|
||||
settings.set_model_override("unsloth/B-GGUF:Q4_K_M", max_seq_length = 8192)
|
||||
assert settings.get_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:q4_k_m", remove = True),
|
||||
"tester",
|
||||
)
|
||||
assert settings.get_model_overrides() == {}
|
||||
assert settings.get_model_override("unsloth/B-GGUF:Q4_K_M") == {}
|
||||
|
||||
|
||||
def test_removal_of_a_path_still_only_touches_the_exact_key(monkeypatch):
|
||||
import routes.settings as settings_route
|
||||
|
||||
_mock_override_store(monkeypatch)
|
||||
settings.set_model_override("/models/foo.gguf", max_seq_length = 8192)
|
||||
settings_route.update_openai_auto_switch_override(
|
||||
settings_route.ModelOverridePayload(model_id = "/models/Foo.gguf", remove = True),
|
||||
"tester",
|
||||
)
|
||||
# A different file must survive its neighbour being forgotten.
|
||||
assert settings.get_model_override("/models/foo.gguf")["max_seq_length"] == 8192
|
||||
|
|
|
|||
|
|
@ -450,24 +450,36 @@ def get_model_override(model_id: str) -> dict:
|
|||
an ambiguous fallback matches nothing, so two POSIX paths differing only in
|
||||
case stay distinct.
|
||||
"""
|
||||
overrides = get_model_overrides()
|
||||
override = overrides.get(model_id)
|
||||
if isinstance(override, dict):
|
||||
return override
|
||||
if not isinstance(model_id, str):
|
||||
key = resolve_model_override_key(model_id)
|
||||
if key is None:
|
||||
return {}
|
||||
override = get_model_overrides().get(key)
|
||||
return override if isinstance(override, dict) else {}
|
||||
|
||||
|
||||
def resolve_model_override_key(model_id: str) -> Optional[str]:
|
||||
"""The stored key an override lookup for ``model_id`` would actually hit.
|
||||
|
||||
Shared by read and remove so "what a load applies" and "what forgetting this
|
||||
model clears" can never disagree.
|
||||
"""
|
||||
overrides = get_model_overrides()
|
||||
if isinstance(overrides.get(model_id), dict):
|
||||
return model_id
|
||||
if not isinstance(model_id, str):
|
||||
return None
|
||||
# Only repo-style ids fold. A POSIX path is case-sensitive and names a
|
||||
# different file, so matching "/models/Foo.gguf" against an entry saved for
|
||||
# "/models/foo.gguf" would replay another model's context and GPU pin.
|
||||
if _looks_like_filesystem_path(model_id):
|
||||
return {}
|
||||
return None
|
||||
folded = model_id.casefold()
|
||||
matches = [
|
||||
value
|
||||
key
|
||||
for key, value in overrides.items()
|
||||
if isinstance(key, str) and key.casefold() == folded and isinstance(value, dict)
|
||||
]
|
||||
return matches[0] if len(matches) == 1 else {}
|
||||
return matches[0] if len(matches) == 1 else None
|
||||
|
||||
|
||||
def set_model_override(
|
||||
|
|
|
|||
|
|
@ -869,11 +869,13 @@ export function ModelConfigPage({
|
|||
isActiveModel && effectiveAtBaseline && rememberChanged;
|
||||
const defaultConfig = isDefaultConfig(effectiveRuntimeConfig);
|
||||
let saveFailed = false;
|
||||
const evicted: { modelId: string; ggufVariant: string | null }[] = [];
|
||||
if (remember) {
|
||||
saveFailed = !savePerModelConfig(
|
||||
target.id,
|
||||
target.ggufVariant,
|
||||
effectiveRuntimeConfig,
|
||||
evicted,
|
||||
);
|
||||
} else {
|
||||
saveFailed = !deletePerModelConfig(target.id, target.ggufVariant);
|
||||
|
|
@ -896,6 +898,12 @@ export function ModelConfigPage({
|
|||
remember ? effectiveRuntimeConfig : null,
|
||||
);
|
||||
}
|
||||
// Saving can push the local map over budget and silently drop other models.
|
||||
// Their server entries would otherwise keep being applied by API loads with
|
||||
// nothing left in the UI showing them or able to forget them.
|
||||
for (const dropped of evicted) {
|
||||
syncModelOverride(dropped.modelId, dropped.ggufVariant, null);
|
||||
}
|
||||
if (effectivePersistenceOnly) {
|
||||
if (saveFailed) {
|
||||
toast.error("Couldn't save settings for this model.");
|
||||
|
|
|
|||
|
|
@ -217,6 +217,7 @@ function serializedMapEntrySize(key: string, value: StoredMap[string]): number {
|
|||
function deleteOldestEvictableEntry(
|
||||
map: StoredMap,
|
||||
protectedKeys?: ReadonlySet<string>,
|
||||
evicted?: string[],
|
||||
): { key: string; value: StoredMap[string] } | null {
|
||||
for (const key of Object.keys(map)) {
|
||||
// Never evict a future-schema entry an older client cannot interpret.
|
||||
|
|
@ -228,6 +229,7 @@ function deleteOldestEvictableEntry(
|
|||
}
|
||||
const value = map[key];
|
||||
delete map[key];
|
||||
evicted?.push(key);
|
||||
return { key, value };
|
||||
}
|
||||
return null;
|
||||
|
|
@ -236,17 +238,18 @@ function deleteOldestEvictableEntry(
|
|||
function enforceStorageBudget(
|
||||
map: StoredMap,
|
||||
protectedKeys?: ReadonlySet<string>,
|
||||
evicted?: string[],
|
||||
): boolean {
|
||||
let entryCount = Object.keys(map).length;
|
||||
while (entryCount > MAX_ENTRIES) {
|
||||
if (!deleteOldestEvictableEntry(map, protectedKeys)) {
|
||||
if (!deleteOldestEvictableEntry(map, protectedKeys, evicted)) {
|
||||
return false;
|
||||
}
|
||||
entryCount -= 1;
|
||||
}
|
||||
let bytes = serializedMapSize(map);
|
||||
while (bytes > MAX_PER_MODEL_CONFIG_STORAGE_BYTES) {
|
||||
const removed = deleteOldestEvictableEntry(map, protectedKeys);
|
||||
const removed = deleteOldestEvictableEntry(map, protectedKeys, evicted);
|
||||
if (!removed) {
|
||||
return false;
|
||||
}
|
||||
|
|
@ -623,6 +626,13 @@ export function savePerModelConfig(
|
|||
modelId: string,
|
||||
ggufVariant: string | null | undefined,
|
||||
config: PerModelConfig,
|
||||
/**
|
||||
* Receives models dropped to stay inside the storage budget. Eviction is
|
||||
* silent and still reports success, so without this their server-side
|
||||
* overrides would keep being applied by API loads with nothing in the UI
|
||||
* still showing them or able to forget them.
|
||||
*/
|
||||
evicted?: { modelId: string; ggufVariant: string | null }[],
|
||||
): boolean {
|
||||
if (
|
||||
typeof config.chatTemplateOverride === "string" &&
|
||||
|
|
@ -646,10 +656,22 @@ export function savePerModelConfig(
|
|||
const [key] = storageKeysForModelVariant(modelId, ggufVariant);
|
||||
deleteConfigEntriesForModelVariant(map, modelId, ggufVariant);
|
||||
map[key] = toStoredConfig(normalized);
|
||||
if (!enforceStorageBudget(map, new Set([key]))) {
|
||||
const evictedKeys: string[] = [];
|
||||
if (!enforceStorageBudget(map, new Set([key]), evictedKeys)) {
|
||||
return false;
|
||||
}
|
||||
return writeMap(map);
|
||||
const written = writeMap(map);
|
||||
if (written && evicted) {
|
||||
for (const evictedKey of evictedKeys) {
|
||||
const id = modelIdFromStorageKey(evictedKey);
|
||||
if (!id) {
|
||||
continue;
|
||||
}
|
||||
const variant = ggufVariantFromStorageKey(evictedKey);
|
||||
evicted.push({ modelId: id, ggufVariant: variant ? variant : null });
|
||||
}
|
||||
}
|
||||
return written;
|
||||
}
|
||||
|
||||
/** Every saved per-model config, decoded back to the ids it was keyed by. */
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue