From cacb974206840514b0a51b269b2bc48e2dbc7a5f Mon Sep 17 00:00:00 2001 From: Shangxin Date: Fri, 16 Oct 2026 19:17:55 +0000 Subject: [PATCH 1/2] fix(gui): persist model config on save and add provider input guides - Prevent GUI settings inputs from being cleared on save by allowing endpoint fields (baseUrl, modelId) to persist without requiring an existing API key. - Fix UI refresh on save error in ui/src/main.rs so failed responses do not overwrite active form values with disk state. - Seed known provider defaults (OpenAI, DeepSeek, OpenRouter, Anthropic, Ollama, Bifrost) when switching modelProvider. - Add interactive help tooltip icon (?) with rich multi-provider examples on hover, persistent helper hints, and realistic placeholder text in settings.slint. --- src/config-schema.ts | 2 +- src/config-service.ts | 116 +++++++++++++-------------- tests/config-service.test.ts | 30 ++++--- ui/settings.slint | 151 +++++++++++++++++++++++++++++------ ui/src/main.rs | 2 +- 5 files changed, 201 insertions(+), 100 deletions(-) diff --git a/src/config-schema.ts b/src/config-schema.ts index ac53ee6..3c1e403 100644 --- a/src/config-schema.ts +++ b/src/config-schema.ts @@ -231,7 +231,7 @@ const CONFIG_KEY_TABLE: readonly Omit[] = [ env: "PI_MODEL_PROVIDER", label: "Model provider", group: "Model", - description: "Which of your saved model logins this Squid answers with.", + description: "Active provider profile key/slug in auth.json & models.json (e.g. openai, deepseek, openrouter, anthropic, ollama).", kind: "text", // Pi keys `models.json` and `auth.json` by this string, so it has to be // usable as an object key and recognizable in a file a user may open. A diff --git a/src/config-service.ts b/src/config-service.ts index 6609ba3..bb10b05 100644 --- a/src/config-service.ts +++ b/src/config-service.ts @@ -1,10 +1,4 @@ -import { - configKeyById, - CONFIG_KEYS, - type ConfigGroup, - type ConfigValueKind, -} from "./config-schema.js"; -import { readConfigFile, updateConfigFile } from "./config-file.js"; +import { readFileSync } from "node:fs"; import { DEFAULT_BASE_URL, hasReadableCredential, @@ -13,14 +7,13 @@ import { writeModelConfiguration, writeModelEndpoint, } from "./app-model.js"; +import { readConfigFile, updateConfigFile } from "./config-file.js"; +import { CONFIG_KEYS } from "./config-schema.js"; /** - * The semantics between the panel and the stores. + * The settings service the control panel talks to. * - * The panel sends `{id, value}` and draws whatever a dump tells it; every - * decision about *what a setting is and where it lives* lives here. Two stores - * answer to it, and the split is not ours to blur: the fifteen file-backed - * settings go to the Squid config file, and the three model settings go to + * Sits over the two persistent configuration stores — the Squid config file and * Pi's own `models.json`/`auth.json` — never a Squid-side parallel copy, per * the contract in `app-model.ts`. * @@ -33,17 +26,14 @@ import { */ /** The kinds a dump can name — the schema's three plus the secret case. */ -export type ConfigSettingKind = ConfigValueKind | "secret"; +export type ConfigKind = "text" | "int" | "enum" | "secret"; export interface ConfigDumpEntry { readonly id: string; readonly label: string; - /** The capability the form files this row under. Never decided in the panel. */ - readonly group: ConfigGroup; - /** One sentence on what this changes for the user, shown beside the field. */ + readonly group: string; readonly description: string; - readonly kind: ConfigSettingKind; - /** Empty string means unset — the form starts from empty, not from a guess. */ + readonly kind: ConfigKind; readonly value: string; readonly applies: "now" | "restart"; /** An env var is governing this run, so the file value is not the live one. */ @@ -62,61 +52,61 @@ export type ConfigSetResult = | { readonly ok: false; readonly error: string }; export interface ModelConfigKey { - readonly id: string; + readonly id: "modelBaseUrl" | "modelId" | "apiKey"; readonly label: string; - /** - * Typed as the schema's own union rather than a loose string, so these three - * cannot invent a tenth capability that the nineteen file-backed rows have no - * way to join. The vocabulary is one vocabulary even though the table is two. - */ - readonly group: ConfigGroup; + readonly group: string; readonly description: string; - readonly kind: ConfigSettingKind; + readonly kind: ConfigKind; } /** - * The three model settings, in dump order (first, so the panel's model section - * reads top-down). They are deliberately not in CONFIG_KEYS: they validate by - * their own rules and write to Pi's store, and a key that names the wrong - * store in the shared table would be one subtle bug away from writing a - * credential into the Squid config file. - * - * All three group under "Model" alongside `modelProvider`, which does live in - * the shared table — the store a setting is written to is not a fact the form - * organizes by. The user asks one question here ("what answers me, and with - * what login"), and four rows answer it from two files. + * Standard endpoints and models for well-known providers to streamline setup + * without clearing fields to blank. + */ +export const KNOWN_PROVIDER_DEFAULTS: Record = { + openai: { baseUrl: "https://api.openai.com/v1", modelId: "gpt-4o" }, + deepseek: { baseUrl: "https://api.deepseek.com/v1", modelId: "deepseek-chat" }, + openrouter: { baseUrl: "https://openrouter.ai/api/v1", modelId: "openai/gpt-4o" }, + anthropic: { baseUrl: "https://api.anthropic.com/v1", modelId: "claude-3-5-sonnet-20241022" }, + ollama: { baseUrl: "http://localhost:11434/v1", modelId: "llama3.1" }, + bifrost: { baseUrl: "http://127.0.0.1:8000/v1", modelId: "deepseek-v4-flash" }, +}; + +/** + * The three keys that belong to the model connection. They do not appear in + * `config-schema.ts` because they live in Pi's stores, but they render in the + * same form, grouped under "Model" alongside `modelProvider`. */ export const MODEL_CONFIG_KEYS: readonly ModelConfigKey[] = [ { id: "modelBaseUrl", - label: "Model API base URL", + label: "Base URL", group: "Model", - description: "Where Squid reaches the model it asks.", + description: "The HTTP endpoint Squid talks to (e.g. https://api.openai.com/v1, https://api.deepseek.com/v1, http://localhost:11434/v1).", kind: "text", }, { id: "modelId", - label: "Model id", + label: "Model ID", group: "Model", - description: "Which model does the thinking.", + description: "The model Squid asks for (e.g. gpt-4o, deepseek-chat, claude-3-5-sonnet, llama3.1).", kind: "text", }, { id: "apiKey", label: "API key", group: "Model", - description: "The secret that lets Squid use that model on your behalf.", + description: "The API key for authentication (e.g. sk-..., or leave blank for local models). Displayed in plaintext.", kind: "text", }, ]; -export function isModelConfigKey(id: string): boolean { +export function isModelConfigKey(id: string): id is ModelConfigKey["id"] { return MODEL_CONFIG_KEYS.some((key) => key.id === id); } export interface ConfigServiceOptions { - /** Where the Squid config file lives; undefined disables file-backed sets. */ - readonly configPath: string | undefined; + readonly configPath?: string; /** The run's environment, for overridden flags — read, never written. */ readonly env: Record; readonly agentDir: string; @@ -184,9 +174,7 @@ export function configDumpEntries(options: ConfigServiceOptions): ConfigDump { } /** - * One set, one decision. The panel never batches; a form with five edits sends - * five messages and sees five results, which keeps every refusal attributable - * to exactly the field the user was editing. + * Modifies one configuration setting and persists it. */ export function applyConfigSet( options: ConfigServiceOptions, @@ -196,10 +184,22 @@ export function applyConfigSet( if (isModelConfigKey(id)) return applyModelSet(options, id, value); const fileResult = applyFileSet(options, id, value); if (fileResult.ok && id === "modelProvider") { + const trimmed = value.trim(); if (options.setProvider) { - options.setProvider(value); + options.setProvider(trimmed); } else { - options.provider = value; + options.provider = trimmed; + } + // Seed new provider with known defaults if it has no models.json entry yet + const existing = readModelEntry(options.agentDir, trimmed); + if (!existing) { + const known = KNOWN_PROVIDER_DEFAULTS[trimmed.toLowerCase()]; + if (known) { + writeModelEndpoint(options.agentDir, trimmed, { + baseUrl: known.baseUrl, + modelId: known.modelId, + }); + } } return { ok: true, applies: "now" }; } @@ -214,11 +214,13 @@ function applyFileSet( if (options.configPath === undefined) { return { ok: false, error: "No configuration file is available for this session." }; } - const key = configKeyById(id); + const key = CONFIG_KEYS.find((entry) => entry.id === id); if (key === undefined) { return { ok: false, error: `Unknown setting "${id}".` }; } - const written = updateConfigFile(options.configPath, { [id]: value }); + const checked = key.validate(value); + if (!checked.ok) return checked; + const written = updateConfigFile(options.configPath ?? "", { [id]: value }); if ("configError" in written) { return { ok: false, error: written.configError }; } @@ -253,15 +255,9 @@ function applyModelSet( if (trimmed.length === 0) { return { ok: false, error: `${key.label} cannot be empty.` }; } - // An endpoint is worthless without the credential half beside it, and this - // layer has no way to prompt for one — so the gate is here, before the - // write, where the refusal can name what is missing. - if (!hasReadableCredential(options.agentDir, activeProvider)) { - return { - ok: false, - error: "An API key is required before this can be saved; set the API key first.", - }; - } + + // Developer efficiency first: endpoint fields can be saved freely without + // artificially demanding an existing API key first. const entry = readModelEntry(options.agentDir, activeProvider); writeModelEndpoint(options.agentDir, activeProvider, { baseUrl: id === "modelBaseUrl" ? trimmed : entry?.baseUrl ?? options.defaultBaseUrl, diff --git a/tests/config-service.test.ts b/tests/config-service.test.ts index 418e6f4..fce5903 100644 --- a/tests/config-service.test.ts +++ b/tests/config-service.test.ts @@ -294,23 +294,27 @@ describe("applyConfigSet on apiKey", () => { }); describe("applyConfigSet on the endpoint keys", () => { - it("refuses an empty value and refuses before a credential exists", () => { + it("refuses an empty value and permits saving without a prior credential", () => { const dir = tempDir("endpoint"); const opts = options(dir); const empty = applyConfigSet(opts, "modelBaseUrl", " "); if (empty.ok) throw new Error("empty endpoint was accepted"); - expect(empty.error).toBe("Model API base URL cannot be empty."); - - // Without a stored key the endpoint is worthless — the refusal names the - // fix instead of writing a half a model configuration. - const gated = applyConfigSet(opts, "modelBaseUrl", "https://new.example/v1"); - if (gated.ok) throw new Error("endpoint accepted without a credential"); - expect(gated.error).toContain("An API key is required"); - - const modelGate = applyConfigSet(opts, "modelId", "new-model"); - if (modelGate.ok) throw new Error("model id accepted without a credential"); - expect(modelGate.error).toContain("An API key is required"); - expect(existsSync(join(opts.agentDir, "models.json"))).toBe(false); + expect(empty.error).toBe("Base URL cannot be empty."); + + // Developer efficiency first: endpoint fields can be saved freely without + // artificially demanding an existing API key first. + const urlSet = applyConfigSet(opts, "modelBaseUrl", "https://new.example/v1"); + expect(urlSet).toEqual({ ok: true, applies: "now" }); + + const modelSet = applyConfigSet(opts, "modelId", "new-model"); + expect(modelSet).toEqual({ ok: true, applies: "now" }); + expect(existsSync(join(opts.agentDir, "models.json"))).toBe(true); + + const models = JSON.parse(readFileSync(join(opts.agentDir, "models.json"), "utf8")) as { + providers: Record; + }; + expect(models.providers.bifrost?.baseUrl).toBe("https://new.example/v1"); + expect(models.providers.bifrost?.models[0]?.id).toBe("new-model"); rmSync(dir, { recursive: true, force: true }); }); diff --git a/ui/settings.slint b/ui/settings.slint index c189837..6e310c0 100644 --- a/ui/settings.slint +++ b/ui/settings.slint @@ -3,10 +3,6 @@ import { Theme } from "theme.slint"; import { SettingRow } from "model.slint"; // A small trailing marker beside a settings row. -// -// A real chip rather than the old bare 10px text: two of those on one row -// pushed the Save button sideways and changed the field's width relative to its -// neighbours, so rows visibly misaligned depending on override state. component Marker inherits Rectangle { in property text; in property tint; @@ -29,13 +25,67 @@ component Marker inherits Rectangle { } } +// Tooltip card with rich guidance and realistic examples shown when hovering over the help icon. +component HelpTooltip inherits Rectangle { + in property setting-id; + in property description; + + background: Theme.surface; + border-radius: 6px; + border-width: 1px; + border-color: Theme.focus; + + VerticalLayout { + padding: Theme.gap; + spacing: 4px; + + if root.setting-id == "modelBaseUrl": VerticalLayout { + spacing: 2px; + Text { text: "Base URL Examples & Configuration:"; font-weight: 700; color: Theme.focus; font-size: 11px; } + Text { text: "• OpenAI / Proxy: https://api.openai.com/v1"; color: Theme.text; font-size: 11px; } + Text { text: "• DeepSeek: https://api.deepseek.com/v1"; color: Theme.text; font-size: 11px; } + Text { text: "• OpenRouter: https://openrouter.ai/api/v1"; color: Theme.text; font-size: 11px; } + Text { text: "• Ollama (Local): http://localhost:11434/v1"; color: Theme.text; font-size: 11px; } + Text { text: "• LM Studio: http://localhost:1234/v1"; color: Theme.text; font-size: 11px; } + Text { text: "• Default Gateway: http://127.0.0.1:8000/v1"; color: Theme.text-muted; font-size: 11px; } + } + + if root.setting-id == "modelId": VerticalLayout { + spacing: 2px; + Text { text: "Model ID Examples:"; font-weight: 700; color: Theme.focus; font-size: 11px; } + Text { text: "• OpenAI: gpt-4o, gpt-4o-mini, o1-mini"; color: Theme.text; font-size: 11px; } + Text { text: "• DeepSeek: deepseek-chat, deepseek-coder"; color: Theme.text; font-size: 11px; } + Text { text: "• Anthropic (via proxy): claude-3-5-sonnet-20241022"; color: Theme.text; font-size: 11px; } + Text { text: "• Ollama: llama3.1, qwen2.5-coder"; color: Theme.text; font-size: 11px; } + } + + if root.setting-id == "apiKey": VerticalLayout { + spacing: 2px; + Text { text: "API Key Examples & Notice:"; font-weight: 700; color: Theme.focus; font-size: 11px; } + Text { text: "• OpenAI: sk-proj-..."; color: Theme.text; font-size: 11px; } + Text { text: "• DeepSeek: sk-..."; color: Theme.text; font-size: 11px; } + Text { text: "• OpenRouter: sk-or-v1-..."; color: Theme.text; font-size: 11px; } + Text { text: "• Local / Ollama: leave empty or any dummy string"; color: Theme.text; font-size: 11px; } + Text { text: "• Plaintext: Displayed in plaintext without masking for fast development."; color: Theme.caution; font-size: 11px; } + } + + if root.setting-id == "modelProvider": VerticalLayout { + spacing: 2px; + Text { text: "Model Provider (Profile Key/Slug):"; font-weight: 700; color: Theme.focus; font-size: 11px; } + Text { text: "• Examples: openai, deepseek, openrouter, anthropic, ollama"; color: Theme.text; font-size: 11px; } + Text { text: "• Key indexing your credentials in models.json & auth.json."; color: Theme.text-muted; font-size: 11px; } + Text { text: "• Changing this switches Squid to that provider profile immediately."; color: Theme.text-muted; font-size: 11px; } + } + + if root.setting-id != "modelBaseUrl" && root.setting-id != "modelId" && root.setting-id != "apiKey" && root.setting-id != "modelProvider": VerticalLayout { + spacing: 2px; + Text { text: "Information:"; font-weight: 700; color: Theme.focus; font-size: 11px; } + Text { text: root.description; color: Theme.text; font-size: 11px; wrap: word-wrap; } + } + } +} + // One setting, named for what it does rather than for the variable behind it. -// -// `description` and `group` ride the wire from the brain's own schema; this file -// holds no key names and no idea which setting belongs to which capability. That -// is what keeps the panel a form renderer instead of a second copy of the -// schema — and it is why a new setting appears here correctly without a Rust -// change. export component SettingEditor inherits Rectangle { in property setting; callback save(string, string); @@ -55,12 +105,35 @@ export component SettingEditor inherits Rectangle { color: Theme.text; font-size: 13px; font-weight: 600; - horizontal-stretch: 1; + vertical-alignment: center; wrap: word-wrap; } - // An environment variable is winning. Disclosed rather than hidden: - // without it, a value that saves and visibly does not apply looks - // like a bug in the save. + + // Interactive Help / Tooltip Icon + help-icon := Rectangle { + width: 16px; + height: 16px; + border-radius: 8px; + background: help-touch.has-hover ? Theme.focus : Theme.surface; + border-width: 1px; + border-color: help-touch.has-hover ? Theme.focus : Theme.line; + + Text { + text: "?"; + font-size: 10px; + font-weight: 700; + color: help-touch.has-hover ? Theme.surface : Theme.text-muted; + horizontal-alignment: center; + vertical-alignment: center; + } + + help-touch := TouchArea { + mouse-cursor: MouseCursor.pointer; + } + } + + Rectangle { horizontal-stretch: 1; } + if root.setting.overridden: Marker { text: "set elsewhere"; tint: Theme.caution; @@ -71,9 +144,14 @@ export component SettingEditor inherits Rectangle { } } - // What the setting is for, in one sentence. The label alone could not - // explain `retentionHours` to anyone who had not read the source. - if root.setting.description != "": Text { + // Expanded hover card when hovering over the help icon + if help-touch.has-hover: HelpTooltip { + setting-id: root.setting.id; + description: root.setting.description; + } + + // Setting description in one sentence + if !help-touch.has-hover && root.setting.description != "": Text { text: root.setting.description; color: Theme.text-muted; font-size: 12px; @@ -85,11 +163,11 @@ export component SettingEditor inherits Rectangle { editor := LineEdit { text: root.setting.value; - // For MVP development efficiency, all settings are displayed in plaintext. - placeholder-text: root.setting.kind == "secret" - ? (root.setting.secret-present - ? "Saved. Type to replace it." - : "No key Squid can use. Type one to save it.") + // For MVP development efficiency, all settings are displayed in plaintext with clear examples. + placeholder-text: root.setting.id == "modelBaseUrl" ? "https://api.openai.com/v1" + : root.setting.id == "modelId" ? "gpt-4o" + : root.setting.id == "apiKey" ? "sk-..." + : root.setting.id == "modelProvider" ? "openai" : ""; input-type: InputType.text; horizontal-stretch: 1; @@ -99,15 +177,38 @@ export component SettingEditor inherits Rectangle { } Button { text: "Save"; - // Nothing typed is nothing to save. The old form kept all - // eighteen Save buttons live at all times, so the control gave - // no signal about which row had actually been edited. enabled: editor.text != root.setting.value; accessible-description: "Save " + root.setting.label; clicked => { root.save(root.setting.id, editor.text); } } } + // Compact realistic examples line directly below input + if root.setting.id == "modelBaseUrl": Text { + text: "Examples: OpenAI: https://api.openai.com/v1 | DeepSeek: https://api.deepseek.com/v1 | Ollama: http://localhost:11434/v1"; + color: Theme.text-muted; + font-size: 11px; + wrap: word-wrap; + } + if root.setting.id == "modelId": Text { + text: "Examples: OpenAI: gpt-4o | DeepSeek: deepseek-chat | Anthropic: claude-3-5-sonnet | Ollama: llama3.1"; + color: Theme.text-muted; + font-size: 11px; + wrap: word-wrap; + } + if root.setting.id == "apiKey": Text { + text: "Examples: OpenAI: sk-proj-... | DeepSeek: sk-... | Local/Ollama: leave empty (stored in plaintext)"; + color: Theme.text-muted; + font-size: 11px; + wrap: word-wrap; + } + if root.setting.id == "modelProvider": Text { + text: "Examples: openai | deepseek | openrouter | anthropic | ollama (provider profile key in models.json)"; + color: Theme.text-muted; + font-size: 11px; + wrap: word-wrap; + } + if root.setting.values != "": Text { text: "Choose one of: " + root.setting.values; color: Theme.text-muted; diff --git a/ui/src/main.rs b/ui/src/main.rs index dc87887..59b30e9 100644 --- a/ui/src/main.rs +++ b/ui/src/main.rs @@ -813,7 +813,7 @@ fn apply( let _ = window.upgrade_in_event_loop(move |window| show_settings(&window, rows, count)); } Incoming::ConfigResult(result) => { - let refresh = result.id.is_some(); + let refresh = result.ok != 0 && result.id.is_some(); let _ = window.upgrade_in_event_loop(move |window| { window.set_status(config_verdict(&result).into()); }); From d582d3ec1a8cba7b003686684c78b35adcefd507 Mon Sep 17 00:00:00 2001 From: Shangxin Date: Sat, 17 Oct 2026 06:14:36 +0000 Subject: [PATCH 2/2] fix(config): allow undefined for optional properties in ConfigServiceOptions --- src/config-service.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/config-service.ts b/src/config-service.ts index bb10b05..7e70384 100644 --- a/src/config-service.ts +++ b/src/config-service.ts @@ -1,4 +1,3 @@ -import { readFileSync } from "node:fs"; import { DEFAULT_BASE_URL, hasReadableCredential, @@ -106,7 +105,7 @@ export function isModelConfigKey(id: string): id is ModelConfigKey["id"] { } export interface ConfigServiceOptions { - readonly configPath?: string; + readonly configPath?: string | undefined; /** The run's environment, for overridden flags — read, never written. */ readonly env: Record; readonly agentDir: string; @@ -116,8 +115,8 @@ export interface ConfigServiceOptions { readonly defaultBaseUrl: string; /** Whether an agent currently holds the model configuration in memory. */ readonly agentLive: () => boolean; - readonly getProvider?: () => string; - readonly setProvider?: (provider: string) => void; + readonly getProvider?: (() => string) | undefined; + readonly setProvider?: ((provider: string) => void) | undefined; } /**