diff --git a/studio/frontend/src/features/model-picker/api/migrate-model-overrides.ts b/studio/frontend/src/features/model-picker/api/migrate-model-overrides.ts index 8f6af14e9e..b5f908a109 100644 --- a/studio/frontend/src/features/model-picker/api/migrate-model-overrides.ts +++ b/studio/frontend/src/features/model-picker/api/migrate-model-overrides.ts @@ -8,6 +8,10 @@ // showing as remembered in the UI while an API load quietly uses app defaults, // which is the exact bug the server-side map exists to fix. +import { + normalizeGgufVariantIdentity, + normalizeModelIdentity, +} from "../model-config/model-identity"; import { isDefaultConfig, listPerModelConfigs, @@ -37,6 +41,28 @@ function markRan(): void { } } +/** + * A server key under the same identity this browser stores. + * + * `app_settings` has no schema version and holds whatever id was current when + * the row was written, so an install that predates identity normalization has + * keys like `Unsloth/Repo-GGUF:Q4_K_M` while this browser only ever produces + * the folded form. The backend resolves the two to one model, so an exact + * property lookup would report "not on the server" for a row that is, and the + * backfill would overwrite it. Variants never contain a colon, so the last one + * splits the key; a repo id folds and a POSIX path deliberately does not. + */ +function normalizedOverrideKey(key: string): string { + const separator = key.lastIndexOf(":"); + if (separator < 0) { + return modelOverrideKey(normalizeModelIdentity(key)); + } + return modelOverrideKey( + normalizeModelIdentity(key.slice(0, separator)), + normalizeGgufVariantIdentity(key.slice(separator + 1)), + ); +} + /** * Push local configs the server has never seen. Never deletes and never * overwrites: an entry already on the server is the newer authority, and losing @@ -66,10 +92,17 @@ export async function backfillModelOverrides(): Promise { return; } + const known = new Set(Object.keys(existing).map(normalizedOverrideKey)); + let failed = false; for (const entry of local) { - const key = modelOverrideKey(entry.modelId, entry.ggufVariant); - if (existing[key]) { + // Folded on this side too: a v2 storage key already holds the normalized + // identity, but the older `id::variant` keys this browser still reads back + // hold whatever casing was typed. + const key = normalizedOverrideKey( + modelOverrideKey(entry.modelId, entry.ggufVariant), + ); + if (known.has(key)) { continue; } try { diff --git a/tests/studio/test_model_picker_contracts.py b/tests/studio/test_model_picker_contracts.py index 3634185292..3fa8abb764 100644 --- a/tests/studio/test_model_picker_contracts.py +++ b/tests/studio/test_model_picker_contracts.py @@ -682,3 +682,23 @@ def test_evicted_local_configs_drop_their_server_overrides(): store = " ".join(_read("features/model-picker/model-config/per-model-config.ts").split()) assert "evicted?: { modelId: string; ggufVariant: string | null }[]" in store assert "modelIdFromStorageKey(" in store and "ggufVariantFromStorageKey(" in store + + +def test_backfill_compares_server_keys_by_normalized_identity(): + """app_settings has no schema version, so an install predating identity + normalization holds rows keyed by whatever id was typed, e.g. + "Unsloth/Repo-GGUF:Q4_K_M". This browser only ever stores the folded form, + so an exact property lookup reports "not on the server" for a row that is, + and the one-time backfill then overwrites settings it documents as the newer + authority. The comparison has to fold the same way the backend resolves. + """ + src = " ".join(_read("features/model-picker/api/migrate-model-overrides.ts").split()) + assert "function normalizedOverrideKey(" in src + # Folded on both sides: the older `id::variant` local keys are not folded. + assert "const known = new Set(Object.keys(existing).map(normalizedOverrideKey));" in src + assert "if (known.has(key)) { continue; }" in src + # A variant never holds a colon, so the last one splits the key. Splitting on + # the first would cut a Windows drive letter off every path id. + assert "key.lastIndexOf(\":\")" in src + # Repo ids fold and POSIX paths do not, which is exactly what these do. + assert "normalizeModelIdentity(" in src and "normalizeGgufVariantIdentity(" in src