diff --git a/changelog.d/fixes/15595-engine-config-page-partial-save.md b/changelog.d/fixes/15595-engine-config-page-partial-save.md new file mode 100644 index 000000000000..e1f0ab4b750f --- /dev/null +++ b/changelog.d/fixes/15595-engine-config-page-partial-save.md @@ -0,0 +1 @@ +- **fix(dashboard):** Compression engine pages now save only the fields edited on that page. A save re-reads the stored settings and applies the edits onto the fresh stored copy, so saving an engine page no longer reverts thresholds and tool strategies the Compression Settings tab saved after the page loaded. An emptied text field clears its stored key, `ultra.enabled` survives engine-page saves, and the Ultra page saves on a default install where the model path was never set. (#15595) diff --git a/changelog.d/fixes/15612-engine-config-empty-number-unset.md b/changelog.d/fixes/15612-engine-config-empty-number-unset.md new file mode 100644 index 000000000000..cd826b728344 --- /dev/null +++ b/changelog.d/fixes/15612-engine-config-empty-number-unset.md @@ -0,0 +1 @@ +- **fix(dashboard):** compression engine config pages treat an emptied number field as "not set" instead of saving `0`, which silently changed compression for fields that accept zero (`aggressive.minSavingsThreshold`, `ultra.compressionRate`, `ultra.minScoreThreshold`) and 400'd saves for fields with a floor (`maxTokensPerMessage`, `minRows`, `minBlockChars`, `minChars`, `retrievalRampFactor`). Emptied fields now save as unset, show the schema default as a placeholder, keep an unparseable entry from unsetting the value, and reject overflow input visibly; lite's cap still clears when emptied. diff --git a/src/i18n/messages/en.json b/src/i18n/messages/en.json index 09c72e0c4715..8b9ff481e3f5 100644 --- a/src/i18n/messages/en.json +++ b/src/i18n/messages/en.json @@ -8544,6 +8544,7 @@ "compressionEngineConfig": { "loading": "Loading…", "loadFailed": "Failed to load engine information.", + "settingsLoadFailed": "Failed to load the saved settings. The fields show defaults, and Save stays off until the page reloads.", "engineNotFound": "Engine \"{engine}\" not found.", "saveFailed": "Failed to save configuration.", "previewFailed": "Preview failed.", diff --git a/src/shared/components/compression/EngineConfigForm.tsx b/src/shared/components/compression/EngineConfigForm.tsx index 8be71aede446..e782aadf5472 100644 --- a/src/shared/components/compression/EngineConfigForm.tsx +++ b/src/shared/components/compression/EngineConfigForm.tsx @@ -4,11 +4,11 @@ import type { EngineConfigField } from "@omniroute/open-sse/services/compression export interface EngineConfigFormProps { schema: EngineConfigField[]; value: Record; - onChange: (next: Record) => void; + // Called with one field at a time, so the caller can apply it to its latest state. + onChange: (key: string, value: unknown) => void; } -export function EngineConfigForm({ schema, value, onChange }: EngineConfigFormProps) { - const set = (k: string, v: unknown) => onChange({ ...value, [k]: v }); +export function EngineConfigForm({ schema, value, onChange: set }: EngineConfigFormProps) { return (
{schema.map((f) => { @@ -23,22 +23,19 @@ export function EngineConfigForm({ schema, value, onChange }: EngineConfigFormPr {f.type === "number" && ( - set( - f.key, - f.key === "maxToolLength" && e.target.value === "" - ? Number.NaN - : Number(e.target.value) - ) - } + onChange={(e) => { + // A browser reports badInput with an empty value for unparseable entries + // ("1e", "1,5" in a comma-decimal locale): keep the last valid value rather + // than mapping the entry to the unset sentinel. + if (e.target.value === "" && e.target.validity.badInput) return; + set(f.key, e.target.value === "" ? Number.NaN : Number(e.target.value)); + }} className="border border-border rounded px-2 py-1" /> )} diff --git a/src/shared/components/compression/EngineConfigPage.tsx b/src/shared/components/compression/EngineConfigPage.tsx index 4c05af2caafd..dfb75b79b1be 100644 --- a/src/shared/components/compression/EngineConfigPage.tsx +++ b/src/shared/components/compression/EngineConfigPage.tsx @@ -4,6 +4,13 @@ import { useEffect, useState } from "react"; import { useLocale, useTranslations } from "next-intl"; import type { EngineConfigField } from "@omniroute/open-sse/services/compression/engines/types"; import { EngineConfigForm } from "@/shared/components/compression/EngineConfigForm"; +import { + buildEngineDetailUpdate, + forgetSentEdits, + formAfterSave, + seedEngineForm, + withoutEmptyText, +} from "@/shared/components/compression/engineConfigSave"; // ── Types ───────────────────────────────────────────────────────────────── @@ -119,6 +126,9 @@ export function EngineConfigPage({ engineId }: { engineId: string }) { // ── Data state ────────────────────────────────────────────────────────── const [engine, setEngine] = useState(null); const [configState, setConfigState] = useState>({}); + // The stored values as of the last load or save, in form shape. A save sends the fields + // changed since. + const [savedConfig, setSavedConfig] = useState>({}); const [analytics, setAnalytics] = useState(null); const [loadError, setLoadError] = useState(null); const [loading, setLoading] = useState(true); @@ -156,35 +166,26 @@ export function EngineConfigPage({ engineId }: { engineId: string }) { .catch(() => null) as Promise, ]); - let foundEngine: EngineEntry | null = null; - if (enginesData) { - foundEngine = enginesData.engines?.find((e) => e.id === engineId) ?? null; - } else { - setLoadError(t("loadFailed")); - } + const foundEngine = enginesData?.engines?.find((e) => e.id === engineId) ?? null; // Detailed config lives in the engine's settings sub-object (when it has one); - // the on/off + level moved to the panel. 404/null/missing = schema defaults. + // the on/off + level moved to the panel. A missing sub-object means schema defaults. const subKey = SETTINGS_SUBOBJECT[engineId]; const stored = subKey ? settingsData?.[subKey] : undefined; - const currentConfig: Record = - stored && typeof stored === "object" ? (stored as Record) : {}; if (!cancelled) { + if (!enginesData) { + setLoadError(t("loadFailed")); + } else if (subKey && !settingsData) { + // Without the stored settings the form would show defaults as saved values, so + // Save stays off. + setLoadError(t("settingsLoadFailed")); + } if (analyticsData) setAnalytics(analyticsData); setEngine(foundEngine); - // Seed configState from defaultValues then override with the stored sub-object. - const defaults: Record = {}; - for (const field of foundEngine?.configSchema ?? []) { - defaults[field.key] = field.defaultValue; - } - // Do not seed lite.maxToolLength from the schema default. Persisting 2000 - // would freeze the cap in settings and hide OMNIROUTE_LITE_MAX_TOOL_LENGTH. - // The form still shows 2000 via field.defaultValue until the operator edits it. - if (engineId === "lite" && currentConfig.maxToolLength === undefined) { - delete defaults.maxToolLength; - } - setConfigState({ ...defaults, ...currentConfig }); + const seeded = seedEngineForm(engineId, foundEngine?.configSchema ?? [], stored); + setConfigState(seeded); + setSavedConfig(seeded); setLoading(false); } } @@ -207,38 +208,56 @@ export function EngineConfigPage({ engineId }: { engineId: string }) { setSaveError(null); return; } - // Strip the `enabled` key — engine on/off is the panel's responsibility. - const { enabled: _ignored, ...formDetail } = configState; - void _ignored; - let detail: Record = formDetail; - if (engineId === "lite") { - const raw = formDetail.maxToolLength; - const compressToolResults = formDetail.compressToolResults !== false; - if (!Object.prototype.hasOwnProperty.call(formDetail, "maxToolLength")) { - detail = { compressToolResults }; - } else if (typeof raw === "number" && Number.isFinite(raw)) { - const n = Math.floor(raw); - if (n < 256 || n > 1_000_000) { - setSaveError(t("saveFailed")); - return; - } - detail = { compressToolResults, maxToolLength: n }; - } else { - detail = { compressToolResults, maxToolLength: null }; - } + // Lite's cap must be in range before anything is sent; the save floors it to a whole number. + // Only NaN (the emptied sentinel) skips the check — overflow like 1e999 fails the range below. + const cap = engineId === "lite" ? configState.maxToolLength : undefined; + if ( + typeof cap === "number" && + !Number.isNaN(cap) && + (Math.floor(cap) < 256 || Math.floor(cap) > 1_000_000) + ) { + setSaveError(t("saveFailed")); + return; } + const sent = configState; setSaving(true); setSaveError(null); try { + // The body starts from the copy stored now and changes only the fields edited here. The + // server replaces each sub-object whole (lite merges), so a copy this page loaded earlier + // would write back fields another page saved since. + const current = (await fetch("/api/settings/compression") + .then((r) => (r.ok ? r.json() : null)) + .catch(() => null)) as CompressionSettings | null; + if (!current) { + setSaveError(t("saveFailed")); + return; + } + const detail = buildEngineDetailUpdate(engineId, savedConfig, sent, current[subKey]); const res = await fetch("/api/settings/compression", { method: "PUT", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ [subKey]: detail }), }); if (!res.ok) { + setSavedConfig((saved) => forgetSentEdits(saved, sent)); setSaveError(t("saveFailed")); + return; } + // Show what the server now holds, which can include fields another page changed, and + // keep anything typed while the save was out. + const settings = (await res.json().catch(() => null)) as CompressionSettings | null; + const written = seedEngineForm( + engineId, + engine?.configSchema ?? [], + settings?.[subKey] ?? detail + ); + setSavedConfig(written); + setConfigState((now) => formAfterSave(written, sent, now)); } catch { + // The PUT may have reached the server before the request failed, so the next save + // sends these fields again. + setSavedConfig((saved) => forgetSentEdits(saved, sent)); setSaveError(t("saveFailed")); } finally { setSaving(false); @@ -256,15 +275,15 @@ export function EngineConfigPage({ engineId }: { engineId: string }) { engineId === "headroom" ? { headroom: { - ...(typeof configState.minRows === "number" + ...(typeof configState.minRows === "number" && !Number.isNaN(configState.minRows) ? { minRows: configState.minRows } : {}), }, } : engineId === "aggressive" - ? { aggressive: { ...configState } } + ? { aggressive: withoutEmptyText(configState) } : engineId === "ultra" - ? { ultra: { ...configState } } + ? { ultra: withoutEmptyText(configState) } : undefined; const res = await fetch("/api/compression/preview", { method: "POST", @@ -379,7 +398,7 @@ export function EngineConfigPage({ engineId }: { engineId: string }) { setConfigState((prev) => ({ ...prev, [key]: next }))} /> ) : (

{t("noAdditionalConfiguration")}

@@ -388,7 +407,7 @@ export function EngineConfigPage({ engineId }: { engineId: string }) { {persistable ? (