From f026c640c54b45348dfd148631413c8a5799088a Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Tue, 28 Jul 2026 14:15:13 +0000 Subject: [PATCH] Match server overrides by identity during the one-time backfill app_settings carries no schema version, so an install that predates identity normalization holds rows keyed by whatever id was typed, such as Unsloth/Repo-GGUF:Q4_K_M, while this browser only ever stores the folded form. The exact property lookup therefore reported "not on the server" for a row that is, and the backfill PUT over it, replacing server settings the file documents as the newer authority. The migration runs once on every existing profile, so this lands on exactly the upgrades it was written to protect. Fold both sides before comparing, splitting on the last colon because a quant label never contains one. A repo id and a Windows path fold, a POSIX path does not, which is the same rule the backend resolves by. Verified with the real module under node: the legacy-casing, variant-casing and Windows-path cases go from overwriting to skipping, a second run stays clean, and a genuinely new model, a different quant of the same repo, a POSIX path differing only in case, and a bare legacy key all still migrate. --- .../api/migrate-model-overrides.ts | 37 ++++++++++++++++++- tests/studio/test_model_picker_contracts.py | 20 ++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) 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