From 686b15d319ff76fefdc6e71e55d76980b1b49bf6 Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Mon, 7 Sep 2026 11:29:55 -0400 Subject: [PATCH] fix(frontend): stop a cleared number field being read as zero `Number("")` is 0, so clearing the Context window or Seed box to retype it wrote a real zero into the settings draft. For the context window that is below the server's minimum (`ge=2048`), so the save was rejected and the message talked about a value the user never typed. For the seed it is worse, because 0 is *valid* (`ge=-1`): the save succeeded and silently turned "-1, keep replies varied" into a pinned seed, making every later reply deterministic with nothing on screen to explain it. Ignoring an empty field is not enough on its own. Both inputs are bound directly to the saved numbers, which can never be empty, so the old value snapped straight back and the digits typed next appended to it -- clearing Seed and typing "42" saved -142. My first attempt did exactly that; the retype test below is what caught it. Each field now keeps what the user has literally typed while they are typing, so the box can be empty mid-edit without any number being written, and the override is dropped on blur so the field re-syncs to whatever was actually saved. A parsed value is committed only when the text is a finite number. Co-Authored-By: Claude Opus 5 --- .../features/settings/SettingsPanel.test.tsx | 98 +++++++++++++++++++ .../src/features/settings/SettingsPanel.tsx | 61 +++++++++++- 2 files changed, 157 insertions(+), 2 deletions(-) diff --git a/frontend/src/features/settings/SettingsPanel.test.tsx b/frontend/src/features/settings/SettingsPanel.test.tsx index 04c6ca2..84ec0b2 100644 --- a/frontend/src/features/settings/SettingsPanel.test.tsx +++ b/frontend/src/features/settings/SettingsPanel.test.tsx @@ -355,4 +355,102 @@ describe("SettingsPanel", () => { expect(screen.getByRole("status")).toHaveTextContent("Save the folder setting before downloading"); expect(onDownloadGGUF).not.toHaveBeenCalled(); }); + it("does not read a cleared number field as zero", async () => { + // Number("") is 0. Clearing a field to retype it wrote a real zero into + // the draft: below the server's minimum for the context window, and for + // the seed a *valid* value that silently turned "-1, keep replies varied" + // into a pinned seed. + const user = userEvent.setup(); + const onSave = vi.fn<(settings: CortexSettings) => Promise>().mockResolvedValue(); + const settings: CortexSettings = { + models: { chat: "local-chat:7b", title: null, translation: "translategemma:4b" }, + generation: { temperature: 0.7, num_ctx: 4096, seed: -1, system_instructions: "", bypass_system_prompt: false }, + }; + const models: ModelResponse = { + required_models: [], + optional_models: [], + installed_models: ["local-chat:7b"], + models: [{ name: "local-chat:7b" }], + connection: { success: true, status: "connected", message: "Connected." }, + }; + render( + Promise>().mockResolvedValue()} + onReplaceMemory={vi.fn<(memos: string[]) => Promise>().mockResolvedValue()} + onClearMemory={vi.fn<() => Promise>().mockResolvedValue()} + models={models} + modelBusy={false} + modelProgress={null} + setupUrl="https://ollama.com/download" + onCheckModels={vi.fn<() => Promise>().mockResolvedValue()} + onPullModel={vi.fn<(model: string) => Promise>().mockResolvedValue()} + llamacppStatus={{ state: "idle", binary_present: false, loaded_model: null, last_error: null, models_directory: "" }} + onDownloadGGUF={vi.fn().mockResolvedValue(undefined)} + onClose={vi.fn()} + />, + ); + + await user.click(screen.getByRole("button", { name: /AI Model/ })); + await user.clear(screen.getByLabelText("Context window")); + await user.clear(screen.getByLabelText("Seed")); + await user.click(screen.getByRole("button", { name: "Save settings" })); + + await waitFor(() => expect(onSave).toHaveBeenCalled()); + expect(onSave).toHaveBeenCalledWith(expect.objectContaining({ + generation: expect.objectContaining({ num_ctx: 4096, seed: -1 }), + })); + }); + + it("still accepts a retyped number", async () => { + const user = userEvent.setup(); + const onSave = vi.fn<(settings: CortexSettings) => Promise>().mockResolvedValue(); + const settings: CortexSettings = { + models: { chat: "local-chat:7b", title: null, translation: "translategemma:4b" }, + generation: { temperature: 0.7, num_ctx: 4096, seed: -1, system_instructions: "", bypass_system_prompt: false }, + }; + const models: ModelResponse = { + required_models: [], + optional_models: [], + installed_models: ["local-chat:7b"], + models: [{ name: "local-chat:7b" }], + connection: { success: true, status: "connected", message: "Connected." }, + }; + render( + Promise>().mockResolvedValue()} + onReplaceMemory={vi.fn<(memos: string[]) => Promise>().mockResolvedValue()} + onClearMemory={vi.fn<() => Promise>().mockResolvedValue()} + models={models} + modelBusy={false} + modelProgress={null} + setupUrl="https://ollama.com/download" + onCheckModels={vi.fn<() => Promise>().mockResolvedValue()} + onPullModel={vi.fn<(model: string) => Promise>().mockResolvedValue()} + llamacppStatus={{ state: "idle", binary_present: false, loaded_model: null, last_error: null, models_directory: "" }} + onDownloadGGUF={vi.fn().mockResolvedValue(undefined)} + onClose={vi.fn()} + />, + ); + + await user.click(screen.getByRole("button", { name: /AI Model/ })); + const seed = screen.getByLabelText("Seed"); + await user.clear(seed); + await user.type(seed, "42"); + await user.click(screen.getByRole("button", { name: "Save settings" })); + + await waitFor(() => expect(onSave).toHaveBeenCalled()); + expect(onSave).toHaveBeenCalledWith(expect.objectContaining({ + generation: expect.objectContaining({ seed: 42 }), + })); + }); }); diff --git a/frontend/src/features/settings/SettingsPanel.tsx b/frontend/src/features/settings/SettingsPanel.tsx index 47d7039..0038093 100644 --- a/frontend/src/features/settings/SettingsPanel.tsx +++ b/frontend/src/features/settings/SettingsPanel.tsx @@ -87,6 +87,23 @@ const sections: { id: SettingsSection; label: string; detail: string }[] = [ { id: "system", label: "System", detail: "Runtime and installed models" }, ]; +type NumericField = "num_ctx" | "seed"; + +/** + * Read a number input, or null when it holds nothing usable. + * + * `Number("")` is 0, so clearing a field to retype it used to write a real + * zero into the draft. For the context window that is below the server's + * minimum and the save was rejected; for the seed it is a *valid* value, so + * it silently turned "-1, keep replies varied" into a pinned seed. An empty + * field means the user is mid-edit, not that they chose zero. + */ +function readNumericInput(raw: string): number | null { + if (raw.trim() === "") return null; + const value = Number(raw); + return Number.isFinite(value) ? value : null; +} + export function SettingsPanel({ settings, memos, @@ -132,6 +149,29 @@ export function SettingsPanel({ const update = (next: Partial) => setDraft((current) => ({ ...current, ...next })); + // What the user has literally typed into a number field, while they are + // typing it. The saved settings hold numbers, so a field bound straight to + // them can never be empty -- clearing it to retype snapped the old value + // back and the new digits appended to it ("-1" + "42" = -142). Keeping the + // raw text lets the box be empty mid-edit without ever writing a number + // nobody chose; `commitNumber` drops the override on blur so the field + // re-syncs to whatever was actually saved. + const [numericInputs, setNumericInputs] = useState>>({}); + + const editNumber = (field: NumericField, raw: string, commit: (value: number) => void) => { + setNumericInputs((current) => ({ ...current, [field]: raw })); + const value = readNumericInput(raw); + if (value !== null) commit(value); + }; + + const commitNumber = (field: NumericField) => + setNumericInputs((current) => { + if (current[field] === undefined) return current; + const next = { ...current }; + delete next[field]; + return next; + }); + const chooseChatModel = (chat: string) => update({ models: { ...modelSettings, chat, title: null } }); const setTranslationEnabled = (enabled: boolean) => { @@ -268,10 +308,27 @@ export function SettingsPanel({