From 2fb4e48319dbdcea2a51432390c8ea97dd504ac3 Mon Sep 17 00:00:00 2001 From: DrMelone <27028174+Classic298@users.noreply.github.com> Date: Mon, 13 Apr 2026 01:11:42 +0200 Subject: [PATCH] fix: scope config default fallback to user mode, skip null user values on load merge, submit via component ref - InterfaceSettings.loadSettingsFromSource now only falls through to \$config.default_models when !isAdminMode. In admin-defaults mode the fallback clobbered whatever the admin last saved with the system- wide default on every reopen, so the modal could show (and inadvertently re-save) the wrong model. - Added stripNullValues util and applied it to userUI in +layout.svelte before deepMerge. deepMerge treats source null as an explicit override, so common "unset / inherit" shapes in user settings (textScale, webSearch, etc.) would suppress admin defaults instead of letting them through. Stripping nulls preserves the PR's stated behaviour (defaults apply for fields the user hasn't customized) without changing deepMerge semantics for other callers. - InterfaceSettings exposes submitSettings() and the admin defaults modal now binds the component instance via bind:this and calls submitSettings() through that ref. Replaces the document.getElementById ('tab-interface') lookup, which was ambiguous the moment a second tab-interface form existed in the DOM. --- .../Interface/InterfaceDefaultsModal.svelte | 29 +++++++++++-------- .../components/chat/Settings/Interface.svelte | 15 +++++++++- src/lib/utils/index.ts | 20 +++++++++++++ src/routes/+layout.svelte | 11 +++++-- 4 files changed, 60 insertions(+), 15 deletions(-) diff --git a/src/lib/components/admin/Settings/Interface/InterfaceDefaultsModal.svelte b/src/lib/components/admin/Settings/Interface/InterfaceDefaultsModal.svelte index c853065bb4..4a1275fb47 100644 --- a/src/lib/components/admin/Settings/Interface/InterfaceDefaultsModal.svelte +++ b/src/lib/components/admin/Settings/Interface/InterfaceDefaultsModal.svelte @@ -16,6 +16,11 @@ let loading = false; let saving = false; let adminDefaults: Record = {}; + // Bound via bind:this on the embedded InterfaceSettings so submitHandler + // can flush submit-scoped writes through a specific component instance + // rather than via a document.getElementById('tab-interface') lookup, + // which would be ambiguous as soon as another InterfaceSettings mounted. + let interfaceSettings: { submitSettings?: () => void } | null = null; const saveAdminSettings = async (updates: object) => { adminDefaults = { ...adminDefaults, ...updates }; @@ -48,17 +53,13 @@ const submitHandler = async () => { saving = true; try { - // The embedded InterfaceSettings form has submit-scoped writes - // (updateInterfaceHandler -> saveSettings({ models, imageCompressionSize })) - // that don't fire on per-control change. Request-submit the child - // form first so those fields land in adminDefaults via the - // saveAdminSettings callback, then wait one tick for reactivity - // to flush before we POST. Without this, admins could save - // defaults and silently lose submit-only fields. - const innerForm = document.getElementById('tab-interface') as HTMLFormElement | null; - if (innerForm && typeof innerForm.requestSubmit === 'function') { - innerForm.requestSubmit(); - } + // The embedded InterfaceSettings component exposes submitSettings() + // for this exact flow: it runs the submit-scoped writes (models, + // imageCompressionSize) through saveAdminSettings so they land + // in adminDefaults before we POST. We call it through the + // bind:this ref to avoid the ambiguity of a global + // document.getElementById lookup. + interfaceSettings?.submitSettings?.(); await tick(); await setInterfaceDefaults(localStorage.token, prepareForBackend(adminDefaults)); @@ -111,7 +112,11 @@ -->
- +
diff --git a/src/lib/components/chat/Settings/Interface.svelte b/src/lib/components/chat/Settings/Interface.svelte index c1238d28aa..6bd6adbaf0 100644 --- a/src/lib/components/chat/Settings/Interface.svelte +++ b/src/lib/components/chat/Settings/Interface.svelte @@ -193,6 +193,15 @@ }); }; + // Exported so the admin-defaults modal can flush submit-scoped writes + // (fields that only flow through form submit, not per-control change) + // without reaching into the DOM via document.getElementById('tab-interface'). + // That DOM-global lookup would be ambiguous as soon as another + // InterfaceSettings instance existed on the page. + export const submitSettings = () => { + updateInterfaceHandler(); + }; + const toggleWebSearch = async () => { webSearch = webSearch === null ? 'always' : null; saveSettings({ webSearch: webSearch }); @@ -282,7 +291,11 @@ imageCompressionInChannels = source?.imageCompressionInChannels ?? true; defaultModelId = source?.models?.at(0) ?? ''; - if ($config?.default_models) { + // In admin-defaults mode the $config.default_models fallback would + // clobber whatever the admin last saved with the system-wide default + // on every reopen, so admins could see (and inadvertently re-save) + // the wrong model. Preserve the per-user fallback as-is. + if (!isAdminMode && $config?.default_models) { defaultModelId = $config.default_models.split(',')[0]; } diff --git a/src/lib/utils/index.ts b/src/lib/utils/index.ts index 52d4190ee1..5e0f2a2835 100644 --- a/src/lib/utils/index.ts +++ b/src/lib/utils/index.ts @@ -1864,6 +1864,26 @@ export const displayFileHandler = ( * @param source - The source object (overrides) * @returns A new object with deep-merged properties */ +/** + * Return a copy of `value` with any explicit null properties removed, + * recursing into nested plain objects. Arrays are returned as-is. Useful + * before a deepMerge where the source treats null as "inherit / unset" + * rather than "explicit override" (e.g. merging user settings over admin + * defaults, where null on user means "fall through to the default"). + */ +export const stripNullValues = (value: any): any => { + if (value === null || typeof value !== 'object') return value; + if (Array.isArray(value)) return value; + const result: Record = {}; + for (const key in value) { + if (!Object.prototype.hasOwnProperty.call(value, key)) continue; + const v = value[key]; + if (v === null) continue; + result[key] = stripNullValues(v); + } + return result; +}; + export const deepMerge = (target: any, source: any): any => { // Handle null/undefined cases - distinguish between "not provided" and "explicitly null" if (source === undefined) return target; diff --git a/src/routes/+layout.svelte b/src/routes/+layout.svelte index df5512f3e4..009a3de6f3 100644 --- a/src/routes/+layout.svelte +++ b/src/routes/+layout.svelte @@ -71,7 +71,7 @@ import Spinner from '$lib/components/common/Spinner.svelte'; import { getUserSettings } from '$lib/apis/users'; import { getInterfaceDefaults } from '$lib/apis/configs'; - import { deepMerge } from '$lib/utils'; + import { deepMerge, stripNullValues } from '$lib/utils'; import dayjs from 'dayjs'; import { getChannels } from '$lib/apis/channels'; @@ -957,7 +957,14 @@ } } const userUI = userSettings?.ui ?? localStorageSettings ?? {}; - settings.set(deepMerge(adminDefaults ?? {}, userUI)); + // User settings commonly carry explicit null for "inherit / + // unset" style fields (e.g. textScale, webSearch), and + // deepMerge treats source null as an explicit override — so + // merging userUI directly would suppress the admin default + // for any such field. Strip nulls out of userUI (recursively + // for nested objects) so "no value" really means "inherit + // the admin default" here. + settings.set(deepMerge(adminDefaults ?? {}, stripNullValues(userUI))); setTextScale($settings?.textScale ?? 1); // Set up the token expiry check