diff --git a/docs/src/content/docs/docs/AIAssistant.md b/docs/src/content/docs/docs/AIAssistant.md index a8b33f23..ef0d07ee 100644 --- a/docs/src/content/docs/docs/AIAssistant.md +++ b/docs/src/content/docs/docs/AIAssistant.md @@ -59,6 +59,15 @@ Individual AI Assistant Macro commands can override: - **System prompt**, which overrides the default system prompt for that command. - Advanced model parameters, described in [Advanced sampling settings](#advanced-sampling-settings). +### System prompts are sent as written {#system-prompt-is-literal} + +QuickAdd resolves [Format Syntax](/docs/FormatSyntax/) in the **prompt template** +only. The system prompt - both the default and a command's override - goes to the +model exactly as you typed it, so `{{DATE}}` in a system prompt arrives at the +model as the eight characters `{{DATE}}`. + +Put anything that needs a token in the prompt template instead. + ## Connect a provider {#providers-and-local-models} QuickAdd supports OpenAI-compatible providers, Google Gemini, and Anthropic. diff --git a/src/ai/AIAssistant.systemPromptLiteral.test.ts b/src/ai/AIAssistant.systemPromptLiteral.test.ts new file mode 100644 index 00000000..db558c67 --- /dev/null +++ b/src/ai/AIAssistant.systemPromptLiteral.test.ts @@ -0,0 +1,237 @@ +import { afterAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { TFile } from "obsidian"; +import type { App } from "obsidian"; +import type { AIProvider } from "./Provider"; +import type { CommonResponse } from "./OpenAIRequest"; + +/** + * The invariant behind #1565 / #1568: the AI system prompt is sent to the model + * VERBATIM. Only the prompt (or prompt template) goes through the formatter. + * + * Three modals used to claim otherwise - a live preview resolving the tokens + * plus a `{{` autocomplete offering them - so `{{DATE}}` previewed as a date and + * then reached the model as eight literal characters. The affordance is gone; + * this pins the behaviour it was lying about, in both directions: + * + * - the system prompt arrives at OpenAIRequest byte-identical to the input, and + * - the formatter is never invoked with it. + * + * If #1572 ever makes system prompts formattable, this test fails, and whoever + * changes it is the person who should also restore the preview and the token + * autocomplete in AIAssistantSettingsModal / AIAssistantCommandSettingsModal / + * AIAssistantInfiniteCommandSettingsModal. + */ + +const storeState = vi.hoisted(() => ({ + disableOnlineFeatures: false, +})); + +const mocks = vi.hoisted(() => ({ + makeRequest: vi.fn(), + openAIRequest: vi.fn(), + isLikelyContextLimitError: vi.fn(() => false), + getModelMaxTokens: vi.fn(() => 100000), + getMarkdownFilesInFolder: vi.fn((): unknown[] => []), +})); + +// Reached transitively from Agent -> CompleteFormatter; its real entry point +// `require`s "obsidian", which does not exist outside the app. +vi.mock("obsidian-dataview", () => ({ getAPI: vi.fn() })); + +vi.mock("src/settingsStore", () => ({ + settingsStore: { getState: () => storeState }, +})); + +vi.mock("./OpenAIRequest", () => ({ + OpenAIRequest: mocks.openAIRequest, +})); + +vi.mock("./providerErrors", () => ({ + isLikelyContextLimitError: mocks.isLikelyContextLimitError, +})); + +vi.mock("./aiHelpers", () => ({ + getModelMaxTokens: mocks.getModelMaxTokens, +})); + +vi.mock("src/utilityObsidian", () => ({ + getMarkdownFilesInFolder: mocks.getMarkdownFilesInFolder, +})); + +const { runAIAssistant, Prompt, ChunkedPrompt } = await import("./AIAssistant"); + +vi.stubGlobal("sleep", async () => {}); + +afterAll(() => { + vi.unstubAllGlobals(); +}); + +/** Contains every token shape the removed preview used to resolve on screen. */ +const SYSTEM_PROMPT = + "You are a helpful assistant. Today is {{DATE}} and the note is {{VALUE:title}}."; + +function makeApp(): App { + return {} as App; +} + +function response(content: string): CommonResponse { + return { + id: content, + model: "test-model", + content, + usage: { promptTokens: 1, completionTokens: 1, totalTokens: 2 }, + stopReason: "stop", + stopSequence: null, + created: 0, + }; +} + +function baseSettings() { + return { + apiKey: "key", + model: { name: "test-model", maxTokens: 100000 }, + provider: { + name: "TestProvider", + endpoint: "https://example.test/v1", + apiKey: "", + models: [], + modelSource: "providerApi", + } as AIProvider, + outputVariableName: "output", + showAssistantMessages: false, + systemPrompt: SYSTEM_PROMPT, + modelOptions: {}, + }; +} + +/** Resolves every token, so an accidentally formatted system prompt is obvious. */ +const resolvingFormatter = vi.fn(async (input: string) => + input.replace(/\{\{[^}]*\}\}/g, "RESOLVED"), +); + +/** The `systemPrompt` argument OpenAIRequest was constructed with. */ +function systemPromptSentToProvider(): unknown { + expect(mocks.openAIRequest).toHaveBeenCalled(); + return mocks.openAIRequest.mock.calls[0][4]; +} + +beforeEach(() => { + vi.clearAllMocks(); + storeState.disableOnlineFeatures = false; + mocks.openAIRequest.mockReturnValue(mocks.makeRequest); + mocks.getModelMaxTokens.mockReturnValue(100000); + mocks.makeRequest.mockImplementation(async (prompt: string) => + response(`response:${prompt}`), + ); +}); + +describe("the AI system prompt is sent verbatim", () => { + it("Prompt sends the system prompt unformatted while formatting the prompt", async () => { + await Prompt( + makeApp(), + { ...baseSettings(), prompt: "Summarise {{VALUE:body}}" }, + resolvingFormatter, + ); + + expect(systemPromptSentToProvider()).toBe(SYSTEM_PROMPT); + // The prompt IS formatted - this is the contrast that makes the system + // prompt's exemption a deliberate behaviour rather than a dead code path. + expect(mocks.makeRequest).toHaveBeenCalledWith("Summarise RESOLVED"); + expect(resolvingFormatter).toHaveBeenCalledTimes(1); + expect(resolvingFormatter).not.toHaveBeenCalledWith(SYSTEM_PROMPT); + }); + + it("ChunkedPrompt sends the system prompt unformatted on every chunk request", async () => { + await ChunkedPrompt( + makeApp(), + { + ...baseSettings(), + text: "alpha\nbeta", + promptTemplate: "Chunk: {{VALUE:chunk}}", + chunkSeparator: /\n/g, + resultJoiner: "|", + shouldMerge: false, + maxChunkTokens: 8, + }, + async (_template: string, variables: { [k: string]: unknown }) => + `Chunk: ${String(variables.chunk)}`, + ); + + // One OpenAIRequest is built for the whole run and reused per chunk, so + // checking the constructor argument covers every dispatched request. + expect(systemPromptSentToProvider()).toBe(SYSTEM_PROMPT); + expect(mocks.makeRequest.mock.calls.length).toBeGreaterThan(0); + for (const [prompt] of mocks.makeRequest.mock.calls) { + expect(prompt).not.toContain("RESOLVED"); + } + }); + + it("runAIAssistant formats the prompt TEMPLATE and leaves the system prompt alone", async () => { + // The macro AI Assistant command's path, i.e. the modal that lost its + // preview. Covered explicitly because a partial #1572 implementation could + // format here and nowhere else, leaving a field that IS formatted with a + // note under it saying it is not. + const template = new TFile(); + template.path = "Prompts/Summarise.md"; + template.basename = "Summarise"; + mocks.getMarkdownFilesInFolder.mockReturnValue([template]); + + const app = { + vault: { + getAbstractFileByPath: () => template, + cachedRead: async () => "Summarise {{VALUE:body}}", + }, + } as unknown as App; + + await runAIAssistant( + app, + { + ...baseSettings(), + promptTemplate: { enable: true, name: "Summarise.md" }, + promptTemplateFolder: "Prompts", + }, + resolvingFormatter, + ); + + expect(systemPromptSentToProvider()).toBe(SYSTEM_PROMPT); + expect(mocks.makeRequest).toHaveBeenCalledWith("Summarise RESOLVED"); + expect(resolvingFormatter).toHaveBeenCalledTimes(1); + expect(resolvingFormatter).not.toHaveBeenCalledWith(SYSTEM_PROMPT); + }); + + it("Agent seeds the system message unformatted", async () => { + // `ai.agent()` falls back to the AI settings modal's default system prompt + // when the script passes none, so this path is fed by the same field. + const { Agent } = await import("./tools/Agent"); + const agent = new Agent( + makeApp(), + {} as never, + {} as never, + { model: "test-model", system: SYSTEM_PROMPT }, + ); + + // prompt omitted: buildSeedMessages short-circuits to "" and never builds a + // CompleteFormatter, so this stays a unit test of the system-message seam. + const messages = await ( + agent as unknown as { + buildSeedMessages: (o: object) => Promise< + Array<{ role: string; content: unknown }> + >; + } + ).buildSeedMessages({}); + + expect(messages[0]).toEqual({ role: "system", content: SYSTEM_PROMPT }); + }); + + it("keeps the token characters intact rather than stripping them", async () => { + await Prompt( + makeApp(), + { ...baseSettings(), prompt: "hello" }, + resolvingFormatter, + ); + + // The specific failure mode the removed preview hid: authors saw a date and + // the model saw the eight characters. + expect(systemPromptSentToProvider()).toContain("{{DATE}}"); + }); +}); diff --git a/src/gui/AIAssistantSettingsModal.ts b/src/gui/AIAssistantSettingsModal.ts index a6f8fc81..f86cf0c6 100644 --- a/src/gui/AIAssistantSettingsModal.ts +++ b/src/gui/AIAssistantSettingsModal.ts @@ -1,9 +1,7 @@ import type { App } from "obsidian"; import { Modal, Setting, TextAreaComponent } from "obsidian"; import type { QuickAddSettings } from "src/settings"; -import { FormatSyntaxSuggester } from "./suggesters/formatSyntaxSuggester"; -import { getQuickAddInstance } from "src/quickAddInstance"; -import { FormatDisplayFormatter } from "src/formatters/formatDisplayFormatter"; +import { mountSystemPromptLiteralNote } from "./ai/systemPromptLiteralNote"; import { AIAssistantProvidersModal } from "./AIAssistantProvidersModal"; import { populateModelDropdown } from "./modelSelect"; import { GenericTextSuggester } from "./suggesters/genericTextSuggester"; @@ -149,32 +147,32 @@ export class AIAssistantSettingsModal extends Modal { .setDesc("The default system prompt for the AI Assistant"); const textAreaComponent = new TextAreaComponent(contentEl); - textAreaComponent - .setValue(this.settings.defaultSystemPrompt) - .onChange(async (value) => { - this.settings.defaultSystemPrompt = value; - - formatDisplay.innerText = await displayFormatter.format(value); - }); + textAreaComponent.inputEl.addClass("qa-ai-prompt-textarea"); + // The textarea is appended to contentEl rather than to the Setting's + // controlEl (it needs the full modal width), so nothing associates it with + // the "Default system prompt" name above. + textAreaComponent.inputEl.setAttribute( + "aria-label", + "Default system prompt", + ); - new FormatSyntaxSuggester( - this.app, + // No format preview and no `{{` token autocomplete here: the system prompt + // is sent to the model verbatim (see mountSystemPromptLiteralNote). The + // preview this replaces resolved the tokens on screen and was, for the + // shipped token-free default, a character-for-character duplicate of the + // textarea above it (#1568). + const updateLiteralNote = mountSystemPromptLiteralNote( + contentEl, textAreaComponent.inputEl, - getQuickAddInstance() - ); - const displayFormatter = new FormatDisplayFormatter( - this.app, - getQuickAddInstance() + this.settings.defaultSystemPrompt ?? "", ); - textAreaComponent.inputEl.addClass("qa-ai-prompt-textarea"); - - const formatDisplay = this.contentEl.createEl("span"); - - void (async () => - (formatDisplay.innerText = await displayFormatter.format( - this.settings.defaultSystemPrompt ?? "" - )))(); + textAreaComponent + .setValue(this.settings.defaultSystemPrompt) + .onChange((value) => { + this.settings.defaultSystemPrompt = value; + updateLiteralNote(value); + }); } onClose(): void { diff --git a/src/gui/ChoiceBuilder/components/FormatPreviewField.svelte b/src/gui/ChoiceBuilder/components/FormatPreviewField.svelte index 95567213..92ecfd95 100644 --- a/src/gui/ChoiceBuilder/components/FormatPreviewField.svelte +++ b/src/gui/ChoiceBuilder/components/FormatPreviewField.svelte @@ -34,11 +34,17 @@ let { plugin: QuickAdd; /** * The choice's configured target folder, so {{FOLDER}} / {{FOLDER|name}} - * preview meaningfully. When unknown we fall back to a "Folder/Name" + * preview meaningfully. When `undefined` we fall back to a "Folder/Name" * placeholder rather than an empty string (no caller wires the real path in * yet, but the placeholder keeps the FOLDER token from previewing blank). + * + * `null` means the host has no target folder CONCEPT at all — not "unknown" + * — and {{FOLDER}} previews as the empty string the runtime produces + * (`Formatter.replaceMacrosInString`: `this.targetFolderPath ?? ""`). The + * user-script format option is such a host: nothing there ever creates a note + * in a folder, so the placeholder would invent a path the script cannot get. */ - targetFolderPath?: string; + targetFolderPath?: string | null; } = $props(); /** How long the field must sit still before its problems are shown. */ @@ -86,9 +92,14 @@ $effect(() => { }); // Resolve {{FOLDER}} / {{FOLDER|name}} against the configured target folder, // or a "Folder/Name" placeholder so the token never previews blank when no - // caller wires the real path in (issue: FOLDER preview always empty). + // caller wires the real path in (issue: FOLDER preview always empty). An + // explicit null says this host has no folder at all, and previews "". formatter.setTargetFolderPath( - targetFolderPath?.trim() ? targetFolderPath : "Folder/Name", + targetFolderPath === null + ? null + : targetFolderPath?.trim() + ? targetFolderPath + : "Folder/Name", ); const token = ++previewToken; void (async () => { diff --git a/src/gui/ChoiceBuilder/components/mountFormatPreview.svelte.ts b/src/gui/ChoiceBuilder/components/mountFormatPreview.svelte.ts new file mode 100644 index 00000000..a8707753 --- /dev/null +++ b/src/gui/ChoiceBuilder/components/mountFormatPreview.svelte.ts @@ -0,0 +1,83 @@ +import type { App } from "obsidian"; +import type QuickAdd from "../../../main"; +import { mountComponent, type MountHandle } from "../../svelte/mountComponent"; +import FormatPreviewField from "./FormatPreviewField.svelte"; + +/** + * Hosts the Svelte `FormatPreviewField` in a modal that builds its DOM + * imperatively. + * + * QuickAdd had one format-preview affordance and two implementations. The Svelte + * one is the correct one: labelled, below the field it previews (#1543), a fresh + * formatter per pass, a monotonic token so a slow pass cannot overwrite a newer + * one's result, and the parse warnings collected inline instead of fired as an + * Obsidian Notice per keystroke (#1558/#1560). The imperative copy had none of + * that, and a second imperative fix would have left the same drift in place - so + * the copy is deleted and the component is mounted instead (#1565). + * + * Lives in a `.svelte.ts` module so `$state` is available: mutating a + * `$state`-backed props object is the documented way to feed reactive props to + * an imperatively mounted Svelte 5 component (see `createCommandListProps`, the + * same pattern for `CommandList`). + */ +export interface FormatPreviewHandle { + /** Push the field's current value; call from the input's `onChange`. */ + setValue(value: string): void; + /** + * Unmount the component and remove the host. + * + * The `destroyed` guard is not only about a double `destroy()` (mountComponent + * already guards that): it also makes `setValue` a no-op afterwards. A + * destroyed preview's textarea can still be alive and holding an `onChange` + * closure over this handle - `display()` tears the previews down and only then + * empties `contentEl` - and pushing into an unmounted component's `$state` is + * a write nothing will ever read. + */ + destroy(): void; +} + +export function mountFormatPreview( + container: HTMLElement, + options: { + app: App; + plugin: QuickAdd; + /** The field's value at mount time. */ + value: string; + }, +): FormatPreviewHandle { + // The helper creates and owns its host rather than accepting an arbitrary + // target: `mount()` writes anchor comment nodes into whatever it is given, and + // these modals `createEl` straight onto `contentEl`, so mounting into a shared + // container would interleave the anchors with later `new Setting(...)` rows. + const host = container.ownerDocument.createElement("div"); + host.className = "qa-format-preview-host"; + container.appendChild(host); + + const props = $state({ + value: options.value, + app: options.app, + plugin: options.plugin, + // Explicitly "no folder", not "folder unknown". The builders' "Folder/Name" + // placeholder stands in for a choice's configured target folder; a modal + // field that never creates a note has none, and inventing one would preview + // `Folder/Name/2026-07-27` for a format the runtime resolves to + // `/2026-07-27`. + targetFolderPath: null, + }); + + const mounted: MountHandle = mountComponent(host, FormatPreviewField, props); + let destroyed = false; + + return { + setValue(value: string) { + if (destroyed) return; + props.value = value; + }, + destroy() { + if (destroyed) return; + destroyed = true; + mounted.destroy(); + host.remove(); + }, + }; +} diff --git a/src/gui/MacroGUIs/AIAssistantCommandSettingsModal.ts b/src/gui/MacroGUIs/AIAssistantCommandSettingsModal.ts index 9d88e2bb..3343c36b 100644 --- a/src/gui/MacroGUIs/AIAssistantCommandSettingsModal.ts +++ b/src/gui/MacroGUIs/AIAssistantCommandSettingsModal.ts @@ -1,8 +1,6 @@ import type { App } from "obsidian"; import { ButtonComponent, Modal, Setting, TextAreaComponent, debounce } from "obsidian"; -import { FormatSyntaxSuggester } from "./../suggesters/formatSyntaxSuggester"; -import { getQuickAddInstance } from "src/quickAddInstance"; -import { FormatDisplayFormatter } from "src/formatters/formatDisplayFormatter"; +import { mountSystemPromptLiteralNote } from "../ai/systemPromptLiteralNote"; import type { IAIAssistantCommand } from "src/types/macros/QuickCommands/IAIAssistantCommand"; import { GenericTextSuggester } from "../suggesters/genericTextSuggester"; import { getMarkdownFilesInFolder } from "src/utilityObsidian"; @@ -240,38 +238,34 @@ export class AIAssistantCommandSettingsModal extends Modal { container.appendChild(tokenCountNote); const textAreaComponent = new TextAreaComponent(contentEl); - textAreaComponent - .setValue(this.settings.systemPrompt) - .onChange(async (value) => { - this.settings.systemPrompt = value; - - formatDisplay.innerText = await displayFormatter.format(value); - updateTokenCount(); - }); - - new FormatSyntaxSuggester( - this.app, + textAreaComponent.inputEl.addClass("qa-ai-prompt-textarea"); + // Appended to contentEl rather than the Setting's controlEl (it needs the + // full modal width), so nothing associates it with the name above. + textAreaComponent.inputEl.setAttribute("aria-label", "System prompt"); + + // No format preview and no `{{` token autocomplete here: the system prompt + // is sent to the model verbatim (see mountSystemPromptLiteralNote). The + // token count above is already computed on the raw string, which is the + // same admission. + const updateLiteralNote = mountSystemPromptLiteralNote( + contentEl, textAreaComponent.inputEl, - getQuickAddInstance() - ); - const displayFormatter = new FormatDisplayFormatter( - this.app, - getQuickAddInstance() + this.settings.systemPrompt ?? "", ); - textAreaComponent.inputEl.addClass("qa-ai-prompt-textarea"); - - const formatDisplay = this.contentEl.createEl("span"); const updateTokenCount = debounce(() => { tokenCount.innerText = `Estimated tokens: ${this.systemPromptTokenLength}`; }, 50); - updateTokenCount(); + textAreaComponent + .setValue(this.settings.systemPrompt) + .onChange((value) => { + this.settings.systemPrompt = value; + updateLiteralNote(value); + updateTokenCount(); + }); - void (async () => - (formatDisplay.innerText = await displayFormatter.format( - this.settings.systemPrompt ?? "" - )))(); + updateTokenCount(); } addShowAdvancedSettingsToggle(container: HTMLElement) { diff --git a/src/gui/MacroGUIs/AIAssistantInfiniteCommandSettingsModal.test.ts b/src/gui/MacroGUIs/AIAssistantInfiniteCommandSettingsModal.test.ts index a2844a9f..01f1b5f4 100644 --- a/src/gui/MacroGUIs/AIAssistantInfiniteCommandSettingsModal.test.ts +++ b/src/gui/MacroGUIs/AIAssistantInfiniteCommandSettingsModal.test.ts @@ -1,5 +1,5 @@ -import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; -import { App, Setting } from "obsidian"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { App } from "obsidian"; import { DEFAULT_SETTINGS } from "src/settings"; import { settingsStore } from "src/settingsStore"; import { deepClone } from "src/utils/deepClone"; @@ -45,28 +45,6 @@ function makeSettings(model: string): IInfiniteAIAssistantCommand { } describe("InfiniteAIAssistantCommandSettingsModal max-chunk-tokens", () => { - beforeAll(() => { - // The shared obsidian stub's Setting lacks addSlider; shim it locally so - // the modal can render the Max chunk tokens slider (documented harness gap). - const settingProto = Setting.prototype as unknown as { - addSlider?: (cb: (slider: unknown) => void) => unknown; - }; - settingProto.addSlider ??= function addSlider( - this: unknown, - cb: (slider: unknown) => void, - ) { - const slider = { - setLimits: () => slider, - setValue: () => slider, - setInstant: () => slider, - setDynamicTooltip: () => slider, - onChange: () => slider, - }; - cb(slider); - return this; - }; - }); - beforeEach(() => { settingsStore.replaceState(deepClone(DEFAULT_SETTINGS)); }); diff --git a/src/gui/MacroGUIs/AIAssistantInfiniteCommandSettingsModal.ts b/src/gui/MacroGUIs/AIAssistantInfiniteCommandSettingsModal.ts index 628e146d..0ce31626 100644 --- a/src/gui/MacroGUIs/AIAssistantInfiniteCommandSettingsModal.ts +++ b/src/gui/MacroGUIs/AIAssistantInfiniteCommandSettingsModal.ts @@ -1,8 +1,6 @@ import type { App } from "obsidian"; import { Modal, Setting, TextAreaComponent, debounce } from "obsidian"; -import { FormatSyntaxSuggester } from "./../suggesters/formatSyntaxSuggester"; -import { getQuickAddInstance } from "src/quickAddInstance"; -import { FormatDisplayFormatter } from "src/formatters/formatDisplayFormatter"; +import { mountSystemPromptLiteralNote } from "../ai/systemPromptLiteralNote"; import type { IInfiniteAIAssistantCommand } from "src/types/macros/QuickCommands/IAIAssistantCommand"; import GenericInputPrompt from "../GenericInputPrompt/GenericInputPrompt"; import { estimateTokenCount } from "src/ai/tokenEstimator"; @@ -152,38 +150,34 @@ export class InfiniteAIAssistantCommandSettingsModal extends Modal { container.appendChild(tokenCountNote); const textAreaComponent = new TextAreaComponent(contentEl); - textAreaComponent - .setValue(this.settings.systemPrompt) - .onChange(async (value) => { - this.settings.systemPrompt = value; - - formatDisplay.innerText = await displayFormatter.format(value); - updateTokenCount(); - }); - - new FormatSyntaxSuggester( - this.app, + textAreaComponent.inputEl.addClass("qa-ai-prompt-textarea"); + // Appended to contentEl rather than the Setting's controlEl (it needs the + // full modal width), so nothing associates it with the name above. + textAreaComponent.inputEl.setAttribute("aria-label", "System prompt"); + + // No format preview and no `{{` token autocomplete here: the system prompt + // is sent to the model verbatim (see mountSystemPromptLiteralNote). This + // path is the one that most obviously admitted it - the chunk budget below + // is sized with estimateTokenCount on the raw string. + const updateLiteralNote = mountSystemPromptLiteralNote( + contentEl, textAreaComponent.inputEl, - getQuickAddInstance() - ); - const displayFormatter = new FormatDisplayFormatter( - this.app, - getQuickAddInstance() + this.settings.systemPrompt ?? "", ); - textAreaComponent.inputEl.addClass("qa-ai-prompt-textarea"); - - const formatDisplay = this.contentEl.createEl("span"); const updateTokenCount = debounce(() => { tokenCount.innerText = `Estimated tokens: ${this.systemPromptTokenLength}`; }, 50); - updateTokenCount(); + textAreaComponent + .setValue(this.settings.systemPrompt) + .onChange((value) => { + this.settings.systemPrompt = value; + updateLiteralNote(value); + updateTokenCount(); + }); - void (async () => - (formatDisplay.innerText = await displayFormatter.format( - this.settings.systemPrompt ?? "" - )))(); + updateTokenCount(); } addShowAdvancedSettingsToggle(container: HTMLElement) { diff --git a/src/gui/MacroGUIs/UserScriptSettingsModal.formatPreview.test.ts b/src/gui/MacroGUIs/UserScriptSettingsModal.formatPreview.test.ts new file mode 100644 index 00000000..a86f22fb --- /dev/null +++ b/src/gui/MacroGUIs/UserScriptSettingsModal.formatPreview.test.ts @@ -0,0 +1,355 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { App } from "obsidian"; +import { CommandType } from "../../types/macros/CommandType"; +import type { IUserScript } from "../../types/macros/IUserScript"; +import { UserScriptSettingsModal } from "./UserScriptSettingsModal"; + +/** + * A user script's `type: "format"` option is the one field of the four #1565 + * names that really is a format field - the option type is the script author's + * own declaration, and the shipped example script resolves it with + * `quickAddApi.format()` (docs/public/scripts/EzImport.js). + * + * So it gets the choice builders' preview rather than losing one: labelled, + * below the field, a formatter per pass, a staleness token, and the parse + * warnings inline instead of silence. + */ + +const mocks = vi.hoisted(() => ({ + /** Every FormatDisplayFormatter built, in construction order. */ + instances: [] as Array<{ formatted: string[] }>, + /** Every setTargetFolderPath argument, in call order. */ + targetFolderPaths: [] as Array, + /** Resolves the next format() call manually, to test out-of-order passes. */ + deferrals: [] as Array<() => void>, + deferNext: false, + diagnostics: [] as Array<{ severity: string; message: string }>, +})); + +vi.mock("../../quickAddInstance", () => ({ + getQuickAddInstance: vi.fn(() => ({})), +})); + +vi.mock("../suggesters/formatSyntaxSuggester", () => ({ + FormatSyntaxSuggester: class {}, +})); + +// Full contract, not just format(): FormatPreviewField also calls +// setTargetFolderPath and reads `diagnostics`. +vi.mock("../../formatters/formatDisplayFormatter", () => ({ + FormatDisplayFormatter: class { + private readonly record = { formatted: [] as string[] }; + diagnostics = { + list: () => mocks.diagnostics, + }; + + constructor() { + mocks.instances.push(this.record); + } + + setTargetFolderPath(path: string | null): void { + mocks.targetFolderPaths.push(path); + } + + format(value: string): Promise { + this.record.formatted.push(value); + const resolved = `resolved(${value})`; + if (mocks.deferNext) { + return new Promise((resolve) => { + mocks.deferrals.push(() => resolve(resolved)); + }); + } + return Promise.resolve(resolved); + } + }, +})); + +function createCommand(): IUserScript { + return { + id: "command-1", + name: "Script", + type: CommandType.UserScript, + path: "scripts/script.js", + settings: {}, + }; +} + +function scriptSettings(options: Record) { + return { name: "Script Settings", options }; +} + +function formatOption(defaultValue: string) { + return { + "Note format": { + type: "format" as const, + defaultValue, + placeholder: "Format", + }, + }; +} + +function flush(): Promise { + return new Promise((resolve) => setTimeout(resolve, 0)); +} + +function openModal( + options: Record, +): UserScriptSettingsModal & { contentEl: HTMLElement } { + return new UserScriptSettingsModal( + new App(), + createCommand(), + scriptSettings(options) as ConstructorParameters< + typeof UserScriptSettingsModal + >[2], + ) as UserScriptSettingsModal & { contentEl: HTMLElement }; +} + +function textarea(contentEl: HTMLElement): HTMLTextAreaElement { + const el = contentEl.querySelector( + "textarea.qa-user-script-format-textarea", + ); + if (!el) throw new Error("Format textarea not found"); + return el; +} + +function type(el: HTMLTextAreaElement, value: string) { + el.value = value; + el.dispatchEvent(new Event("input", { bubbles: true })); +} + +beforeEach(() => { + mocks.instances.length = 0; + mocks.targetFolderPaths.length = 0; + mocks.deferrals.length = 0; + mocks.diagnostics.length = 0; + mocks.deferNext = false; +}); + +afterEach(() => { + document.body.innerHTML = ""; +}); + +describe("UserScriptSettingsModal format option preview", () => { + it("renders a labelled preview row BELOW the field it previews", async () => { + const modal = openModal(formatOption("Logged on {{DATE}}")); + await flush(); + + const row = modal.contentEl.querySelector(".qa-preview-row"); + expect(row).not.toBeNull(); + expect(row?.querySelector(".qa-preview-label")?.textContent).toBe( + "Preview: ", + ); + expect(row?.querySelector(".qa-preview-value")?.textContent).toBe( + "resolved(Logged on {{DATE}})", + ); + // #1543: the bare span used to be created before the input and rendered + // above it, where it read as the field's label. + expect( + textarea(modal.contentEl).compareDocumentPosition(row as Node) & + Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + + modal.close(); + }); + + it("builds a formatter per pass rather than memoizing one for the modal", async () => { + const modal = openModal(formatOption("start")); + await flush(); + const afterMount = mocks.instances.length; + + type(textarea(modal.contentEl), "next"); + await flush(); + + // A memoized instance carries a resolved `variables` map into the next + // pass, so an edited option list kept previewing the stale value. + expect(mocks.instances.length).toBeGreaterThan(afterMount); + expect(mocks.instances.at(-1)?.formatted).toEqual(["next"]); + + modal.close(); + }); + + it("drops a stale pass that resolves after a newer one", async () => { + const modal = openModal(formatOption("first")); + await flush(); + + mocks.deferNext = true; + type(textarea(modal.contentEl), "slow"); + await flush(); + type(textarea(modal.contentEl), "fast"); + await flush(); + + expect(mocks.deferrals).toHaveLength(2); + // Resolve the OLDER pass last: without a staleness token it would win. + mocks.deferrals[1](); + await flush(); + mocks.deferrals[0](); + await flush(); + + expect( + modal.contentEl.querySelector(".qa-preview-value")?.textContent, + ).toBe("resolved(fast)"); + + modal.close(); + }); + + it("shows the pass's problems inline instead of firing nothing at all", async () => { + mocks.diagnostics.push({ + severity: "warning", + message: "Unsupported case option 'pasc'", + }); + const modal = openModal(formatOption("{{VALUE:title|case:pasc}}")); + await flush(); + + // Held back until the field has been still (500ms), matching the builders. + expect(modal.contentEl.querySelector(".qa-preview-issue")).toBeNull(); + await new Promise((resolve) => setTimeout(resolve, 600)); + + const issue = modal.contentEl.querySelector(".qa-preview-issue"); + expect(issue?.textContent).toContain("Unsupported case option 'pasc'"); + + modal.close(); + }); + + it("previews {{FOLDER}} as empty, not as an invented folder", async () => { + // The builders fall back to a "Folder/Name" placeholder because a choice HAS + // a configured target folder that no caller wires in yet. A user-script + // option has none at all, and the runtime resolves {{FOLDER}} to "" there + // (Formatter.replaceMacrosInString: `this.targetFolderPath ?? ""`), so the + // placeholder would preview a path the script can never produce. + const modal = openModal(formatOption("{{FOLDER}}/{{DATE}}")); + await flush(); + + expect(mocks.targetFolderPaths).not.toHaveLength(0); + for (const path of mocks.targetFolderPaths) expect(path).toBeNull(); + + modal.close(); + }); + + it("opens for a format option that declares no defaultValue", async () => { + // `initializeUserScriptSettings` deliberately skips options with no + // defaultValue, so `value` reaches addFormatInput as undefined despite its + // `string` type. FormatPreviewField calls `value.trim()` during mount, so an + // uncoerced undefined throws out of display() and out of the constructor - + // and CommandList's `new UserScriptSettingsModal(...).open()` never reaches + // `.open()`, so the gear button silently does nothing. + const modal = openModal({ + "Note format": { type: "format", placeholder: "Format" }, + }); + await flush(); + + expect(textarea(modal.contentEl).value).toBe(""); + // Empty field, so no dangling "Preview:" row. + expect(modal.contentEl.querySelector(".qa-preview-row")).toBeNull(); + + modal.close(); + }); + + it("opens when a stored value is not a string, and shows what the script will get", async () => { + // A script that changed an option from `type: "toggle"` to `type: "format"` + // between versions leaves a boolean in the saved command settings. Blanking + // the field would claim the setting is empty while + // `resolveUserScriptSettings` still forwards `true` to the script. + const command = createCommand(); + command.settings["Note format"] = true; + const modal = new UserScriptSettingsModal( + new App(), + command, + scriptSettings(formatOption("ignored")) as ConstructorParameters< + typeof UserScriptSettingsModal + >[2], + ) as UserScriptSettingsModal & { contentEl: HTMLElement }; + await flush(); + + expect(textarea(modal.contentEl).value).toBe("true"); + expect( + modal.contentEl.querySelector(".qa-preview-value")?.textContent, + ).toBe("resolved(true)"); + // Untouched until the user actually edits the field. + expect(command.settings["Note format"]).toBe(true); + + modal.close(); + }); + + it("ignores setValue after destroy, and destroys idempotently", async () => { + const modal = openModal(formatOption("start")); + const internals = modal as unknown as { + previewHandles: Array<{ + setValue: (v: string) => void; + destroy: () => void; + }>; + }; + await flush(); + const handle = internals.previewHandles[0]; + const passesBefore = mocks.instances.length; + + handle.destroy(); + handle.destroy(); + // The textarea outlives the handle: display() tears the previews down and + // only THEN empties contentEl, so its onChange closure can still fire. + handle.setValue("after destroy"); + await flush(); + + expect(mocks.instances.length).toBe(passesBefore); + expect(document.querySelectorAll(".qa-preview-row")).toHaveLength(0); + + modal.close(); + }); + + it("still writes the edited value into the command settings", async () => { + const command = createCommand(); + const modal = new UserScriptSettingsModal( + new App(), + command, + scriptSettings(formatOption("start")) as ConstructorParameters< + typeof UserScriptSettingsModal + >[2], + ); + await flush(); + + type(textarea(modal.contentEl), "Logged on {{DATE}}"); + + expect(command.settings["Note format"]).toBe("Logged on {{DATE}}"); + + modal.close(); + }); + + it("mounts one preview per format option and tears them all down on close", async () => { + const modal = openModal({ + First: { type: "format", defaultValue: "one", placeholder: "" }, + Second: { type: "format", defaultValue: "two", placeholder: "" }, + }); + await flush(); + + expect(modal.contentEl.querySelectorAll(".qa-preview-row")).toHaveLength(2); + + modal.close(); + expect(document.querySelectorAll(".qa-preview-row")).toHaveLength(0); + }); + + it("does not accumulate previews when display() re-runs", async () => { + const modal = openModal(formatOption("start")); + const internals = modal as unknown as { + display: () => void; + previewHandles: unknown[]; + }; + await flush(); + + // migrateSecretSettings() re-runs display() from a voided promise, which + // empties contentEl. Assert on the tracked handles, NOT on the DOM: an + // orphaned component is detached along with contentEl's children, so it + // disappears from every query while its effects and its 500ms diagnostics + // timer keep running. The handle list is the only thing that can tell the + // difference between torn down and merely invisible. + internals.display(); + await flush(); + internals.display(); + await flush(); + + expect(internals.previewHandles).toHaveLength(1); + expect(modal.contentEl.querySelectorAll(".qa-preview-row")).toHaveLength(1); + + modal.close(); + expect(internals.previewHandles).toHaveLength(0); + expect(document.querySelectorAll(".qa-preview-row")).toHaveLength(0); + }); +}); diff --git a/src/gui/MacroGUIs/UserScriptSettingsModal.ts b/src/gui/MacroGUIs/UserScriptSettingsModal.ts index 03f0b2ac..e47a7d31 100644 --- a/src/gui/MacroGUIs/UserScriptSettingsModal.ts +++ b/src/gui/MacroGUIs/UserScriptSettingsModal.ts @@ -2,7 +2,10 @@ import type { App } from "obsidian"; import { Modal, Notice, Setting, TextAreaComponent } from "obsidian"; import type { IUserScript } from "../../types/macros/IUserScript"; import { getQuickAddInstance } from "../../quickAddInstance"; -import { FormatDisplayFormatter } from "../../formatters/formatDisplayFormatter"; +import { + mountFormatPreview, + type FormatPreviewHandle, +} from "../ChoiceBuilder/components/mountFormatPreview.svelte"; import { FormatSyntaxSuggester } from "../suggesters/formatSyntaxSuggester"; import { setPasswordOnBlur } from "../../utils/setPasswordOnBlur"; import { initializeUserScriptSettings } from "../../utils/userScriptSettings"; @@ -54,6 +57,22 @@ type Option = { description?: string; id?: string } & ( } ); +/** + * The text a `type: "format"` option should render for a stored value that a + * third-party script may have left in any shape. + * + * Absent means absent: `initializeUserScriptSettings` deliberately leaves an + * option with no `defaultValue` unset so the script can apply its own, and + * printing "undefined" into the field would be a lie about what it will receive. + * Anything else is stringified rather than blanked, so the field agrees with the + * value `resolveUserScriptSettings` forwards. + */ +function formatOptionText(value: unknown): string { + if (typeof value === "string") return value; + if (value === undefined || value === null) return ""; + return String(value); +} + function formatTitlePart(value: unknown): string { if (typeof value === "string") return value; if (value === null || value === undefined) return ""; @@ -68,6 +87,9 @@ function formatTitlePart(value: unknown): string { } export class UserScriptSettingsModal extends Modal { + /** One per `type: "format"` option; a script may declare several. */ + private previewHandles: FormatPreviewHandle[] = []; + constructor( app: App, private command: IUserScript, @@ -89,6 +111,10 @@ export class UserScriptSettingsModal extends Modal { protected display() { this.containerEl.addClass("quickAddModal", "userScriptSettingsModal"); + // Emptying contentEl does not stop a Svelte component, it only detaches it, + // so every re-render must tear the previews down explicitly or each orphan + // keeps a live $effect and the preview's 500ms diagnostics timer. + this.destroyPreviews(); this.contentEl.empty(); const titleName = formatTitlePart(this.settings?.name ?? this.command.name); @@ -313,29 +339,71 @@ export class UserScriptSettingsModal extends Modal { private addFormatInput(name: string, value: string, placeholder?: string) { const setting = new Setting(this.contentEl).setName(name); - const formatDisplay = this.contentEl.createEl("span"); + // `value` comes from a third-party script's `settings.options`, so its + // declared `string` is a promise, not a guarantee: an option with no + // `defaultValue` arrives as undefined (initializeUserScriptSettings + // deliberately skips those, so the script can apply its own default), and + // an option whose `type` changed from `toggle` to `format` between script + // versions arrives as a boolean. Coerce here, at the boundary where + // untrusted data enters typed code - FormatPreviewField's `value.trim()` + // runs during mount, so a non-string would throw out of display() and out + // of the constructor, and the gear button in the command list would + // silently do nothing. + // + // A present non-string is STRINGIFIED rather than blanked, so what the + // field shows still corresponds to what `resolveUserScriptSettings` will + // forward to the script. That also keeps this method consistent with its + // four siblings, which all render `value as string` unchanged. Absent + // (undefined/null) renders empty instead of the literal text "undefined", + // which is what a real TextAreaComponent would print. + const text = formatOptionText(value); + const input = new TextAreaComponent(this.contentEl); new FormatSyntaxSuggester(this.app, input.inputEl, getQuickAddInstance()); - const displayFormatter = new FormatDisplayFormatter( - this.app, - getQuickAddInstance() - ); + input.inputEl.addClass("qa-user-script-format-textarea"); + // Appended to contentEl rather than the Setting's controlEl (it needs the + // full modal width), so nothing associates it with the option name above. + input.inputEl.setAttribute("aria-label", name); + + // Mounted AFTER the textarea, so the preview reads as a result of the field + // rather than as its label - it used to be created first and rendered above + // the input, the exact placement #1543 fixed in the choice builders. + const preview = mountFormatPreview(this.contentEl, { + app: this.app, + plugin: getQuickAddInstance(), + value: text, + }); + this.previewHandles.push(preview); input - .setValue(value) - .onChange(async (value) => { + .setValue(text) + .onChange((value) => { this.command.settings[name] = value; - formatDisplay.innerText = await displayFormatter.format(value); + preview.setValue(value); this.onCommandChange?.(); }) .setPlaceholder(placeholder ?? ""); - input.inputEl.addClass("qa-user-script-format-textarea"); + return setting; + } - void (async () => - (formatDisplay.innerText = await displayFormatter.format(value)))(); + /** + * Unmount every mounted preview. + * + * `display()` is re-entrant - the constructor calls it, and + * `migrateSecretSettings()` calls it again from a `void`ed promise that can + * land at any time - and it empties `contentEl`. Emptying the DOM does not + * stop a Svelte component: each orphan would keep a live `$effect` and the + * preview's 500ms diagnostics timer rescheduling forever. + */ + private destroyPreviews(): void { + for (const handle of this.previewHandles) handle.destroy(); + this.previewHandles = []; + } - return setting; + onClose(): void { + this.destroyPreviews(); + super.onClose(); } private async migrateSecretSettings() { diff --git a/src/gui/ai/systemPromptFields.test.ts b/src/gui/ai/systemPromptFields.test.ts new file mode 100644 index 00000000..7ae525b0 --- /dev/null +++ b/src/gui/ai/systemPromptFields.test.ts @@ -0,0 +1,277 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +/** + * The three system-prompt modals must not offer a format affordance: the system + * prompt reaches the model verbatim (pinned by + * AIAssistant.systemPromptLiteral.test.ts), so a live preview resolving its + * tokens asserted a substitution that never happens (#1565), and on the shipped + * token-free default it was a character-for-character duplicate of the textarea + * above it (#1568). + * + * The two mocks below are the load-bearing assertions: they COUNT construction. + * A test that only queried the DOM would keep passing if someone reinstated the + * formatter but rendered it somewhere new. + */ + +const mocks = vi.hoisted(() => ({ + formatDisplayFormatter: vi.fn(), + formatSyntaxSuggester: vi.fn(), +})); + +vi.mock("obsidian-dataview", () => ({ getAPI: vi.fn() })); +vi.mock("src/settingsStore", () => ({ + settingsStore: { + getState: () => ({ + ai: { + promptTemplatesFolderPath: "", + showAssistant: false, + providers: [ + { + id: "test", + name: "TestProvider", + endpoint: "https://example.test/v1", + apiKey: "", + models: [{ name: "gpt-test", maxTokens: 1000 }], + modelSource: "providerApi", + }, + ], + }, + disableOnlineFeatures: false, + }), + }, +})); +vi.mock("src/quickAddInstance", () => ({ + getQuickAddInstance: vi.fn(() => ({})), +})); +vi.mock("src/utilityObsidian", () => ({ + getMarkdownFilesInFolder: vi.fn(() => []), + getAllFolderPathsInVault: vi.fn(() => []), +})); +// Partial: the Infinite modal's chunk-budget slider reaches +// estimateModelInputBudget through aiHelpers, so only the count is stubbed. +vi.mock("src/ai/tokenEstimator", async (importOriginal) => ({ + ...(await importOriginal>()), + estimateTokenCount: vi.fn(() => 0), +})); +vi.mock("src/gui/suggesters/genericTextSuggester", () => ({ + GenericTextSuggester: class {}, +})); +vi.mock("src/formatters/formatDisplayFormatter", () => ({ + FormatDisplayFormatter: class { + constructor(...args: unknown[]) { + mocks.formatDisplayFormatter(...args); + } + async format(input: string) { + return input; + } + }, +})); +vi.mock("src/gui/suggesters/formatSyntaxSuggester", () => ({ + FormatSyntaxSuggester: class { + constructor(...args: unknown[]) { + mocks.formatSyntaxSuggester(...args); + } + }, +})); + +import { App } from "obsidian"; +import type { IAIAssistantCommand } from "src/types/macros/QuickCommands/IAIAssistantCommand"; +import type { IInfiniteAIAssistantCommand } from "src/types/macros/QuickCommands/IAIAssistantCommand"; +import type { QuickAddSettings } from "src/settings"; +import { AIAssistantSettingsModal } from "src/gui/AIAssistantSettingsModal"; +import { AIAssistantCommandSettingsModal } from "src/gui/MacroGUIs/AIAssistantCommandSettingsModal"; +import { InfiniteAIAssistantCommandSettingsModal } from "src/gui/MacroGUIs/AIAssistantInfiniteCommandSettingsModal"; + +const PROSE_PROMPT = "As an AI assistant within Obsidian, help the user."; +const TOKENED_PROMPT = "Today is {{DATE}}. Help the user."; + +function testApp(): App { + const app = new App() as App & { + dom: { appContainerEl: HTMLElement }; + keymap: { pushScope: () => void; popScope: () => void }; + }; + app.dom = { appContainerEl: document.body }; + app.keymap = { pushScope: vi.fn(), popScope: vi.fn() }; + return app; +} + +function aiSettings(defaultSystemPrompt: string): QuickAddSettings["ai"] { + return { + defaultModel: "gpt-test", + defaultSystemPrompt, + promptTemplatesFolderPath: "", + showAssistant: false, + providers: [], + } as unknown as QuickAddSettings["ai"]; +} + +function aiCommand(systemPrompt: string): IAIAssistantCommand { + return { + id: "ai-1", + name: "AI Assistant", + type: "AIAssistant", + model: "gpt-test", + systemPrompt, + outputVariableName: "output", + modelParameters: {}, + promptTemplate: { enable: false, name: "" }, + } as IAIAssistantCommand; +} + +function infiniteCommand(systemPrompt: string): IInfiniteAIAssistantCommand { + return { + ...aiCommand(systemPrompt), + resultJoiner: "\\n", + chunkSeparator: "\\n", + maxChunkTokens: 100, + mergeChunks: false, + } as unknown as IInfiniteAIAssistantCommand; +} + +interface OpenedModal { + contentEl: HTMLElement; + /** Every one of these modals re-renders in place; the AI settings modal does + * it on every "Edit providers", the command modals on every model change. */ + reload: () => void; + close: () => void; + label: string; +} + +function opened( + modal: { contentEl: HTMLElement; close: () => void }, + label: string, +): OpenedModal { + return { + contentEl: modal.contentEl, + reload: () => (modal as unknown as { reload: () => void }).reload(), + close: () => modal.close(), + label, + }; +} + +/** Each modal, paired with a factory that opens it. */ +const MODALS: Array<{ + name: string; + open: (systemPrompt: string) => OpenedModal; +}> = [ + { + name: "AIAssistantSettingsModal (default system prompt)", + open: (systemPrompt) => + opened( + new AIAssistantSettingsModal(testApp(), aiSettings(systemPrompt)), + "Default system prompt", + ), + }, + { + name: "AIAssistantCommandSettingsModal (system prompt)", + open: (systemPrompt) => + opened( + new AIAssistantCommandSettingsModal(testApp(), aiCommand(systemPrompt)), + "System prompt", + ), + }, + { + name: "InfiniteAIAssistantCommandSettingsModal (system prompt)", + open: (systemPrompt) => + opened( + new InfiniteAIAssistantCommandSettingsModal( + testApp(), + infiniteCommand(systemPrompt), + ), + "System prompt", + ), + }, +]; + +function promptTextarea(contentEl: HTMLElement): HTMLTextAreaElement { + const textarea = contentEl.querySelector( + "textarea.qa-ai-prompt-textarea", + ); + if (!textarea) throw new Error("System prompt textarea not found"); + return textarea; +} + +describe("AI system-prompt fields offer no format affordance", () => { + beforeEach(() => { + vi.clearAllMocks(); + document.body.innerHTML = ""; + }); + + for (const { name, open } of MODALS) { + describe(name, () => { + it("builds no preview formatter and no token autocomplete", () => { + const { contentEl, close } = open(PROSE_PROMPT); + + expect(mocks.formatDisplayFormatter).not.toHaveBeenCalled(); + expect(mocks.formatSyntaxSuggester).not.toHaveBeenCalled(); + // #1568: no bare span echoing the prompt back under the field. + expect(contentEl.textContent).not.toContain(PROSE_PROMPT); + expect(promptTextarea(contentEl).value).toBe(PROSE_PROMPT); + + close(); + }); + + it("stays quiet for a prose prompt and explains itself once a token appears", () => { + const { contentEl, close } = open(PROSE_PROMPT); + const note = contentEl.querySelector(".qa-literal-format-note"); + expect(note).not.toBeNull(); + expect( + note?.classList.contains("qa-literal-format-note--shown"), + ).toBe(false); + + const textarea = promptTextarea(contentEl); + textarea.value = TOKENED_PROMPT; + textarea.dispatchEvent(new Event("input", { bubbles: true })); + + expect( + note?.classList.contains("qa-literal-format-note--shown"), + ).toBe(true); + + close(); + }); + + it("shows the note immediately when the stored prompt already has a token", () => { + const { contentEl, close } = open(TOKENED_PROMPT); + + expect( + contentEl + .querySelector(".qa-literal-format-note") + ?.classList.contains("qa-literal-format-note--shown"), + ).toBe(true); + + close(); + }); + + it("names the textarea, which sits outside its Setting row", () => { + const { contentEl, close, label } = open(PROSE_PROMPT); + + expect(promptTextarea(contentEl).getAttribute("aria-label")).toBe( + label, + ); + + close(); + }); + + it("keeps exactly one note, still correct, across a reload", () => { + // reload() empties contentEl and rebuilds. A note hoisted out of the + // field's own builder would survive the first render and vanish here, + // silently taking the field's last remaining signal with it. + const { contentEl, close, reload } = open(TOKENED_PROMPT); + reload(); + + const notes = contentEl.querySelectorAll(".qa-literal-format-note"); + expect(notes).toHaveLength(1); + expect( + notes[0].classList.contains("qa-literal-format-note--shown"), + ).toBe(true); + expect(promptTextarea(contentEl).getAttribute("aria-describedby")).toBe( + notes[0].id, + ); + expect(mocks.formatDisplayFormatter).not.toHaveBeenCalled(); + expect(mocks.formatSyntaxSuggester).not.toHaveBeenCalled(); + + close(); + }); + }); + } +}); diff --git a/src/gui/ai/systemPromptLiteralNote.test.ts b/src/gui/ai/systemPromptLiteralNote.test.ts new file mode 100644 index 00000000..8c68dc02 --- /dev/null +++ b/src/gui/ai/systemPromptLiteralNote.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, it } from "vitest"; +import { mountSystemPromptLiteralNote } from "./systemPromptLiteralNote"; + +function host(): { container: HTMLElement; field: HTMLTextAreaElement } { + const container = document.createElement("div"); + const field = document.createElement("textarea"); + container.appendChild(field); + document.body.appendChild(container); + return { container, field }; +} + +function noteOf(container: HTMLElement): HTMLElement { + const note = container.querySelector(".qa-literal-format-note"); + expect(note).not.toBeNull(); + return note as HTMLElement; +} + +/** The class the stylesheet keys `display: block` off. */ +function isShown(container: HTMLElement): boolean { + return noteOf(container).classList.contains("qa-literal-format-note--shown"); +} + +describe("mountSystemPromptLiteralNote", () => { + it("stays hidden for a prose prompt", () => { + const { container, field } = host(); + mountSystemPromptLiteralNote( + container, + field, + "As an AI assistant within Obsidian, your primary goal is to help users.", + ); + + expect(isShown(container)).toBe(false); + }); + + it("shows for a prompt that already contains a token", () => { + const { container, field } = host(); + mountSystemPromptLiteralNote(container, field, "Today is {{DATE}}."); + + expect(isShown(container)).toBe(true); + }); + + it("follows the value as the field is edited, in both directions", () => { + const { container, field } = host(); + const update = mountSystemPromptLiteralNote(container, field, ""); + + expect(isShown(container)).toBe(false); + update("You are helpful."); + expect(isShown(container)).toBe(false); + update("You are helpful. Today is {{DATE}}."); + expect(isShown(container)).toBe(true); + update("You are helpful."); + expect(isShown(container)).toBe(false); + }); + + it("describes the field only while the note is visible", () => { + // An accessible description may include a directly-referenced HIDDEN node, + // so a permanent reference would describe the field with a line the sighted + // user cannot see. + const { container, field } = host(); + const update = mountSystemPromptLiteralNote(container, field, ""); + + expect(field.hasAttribute("aria-describedby")).toBe(false); + + update("Today is {{DATE}}."); + const id = field.getAttribute("aria-describedby"); + expect(id).toBeTruthy(); + expect(noteOf(container).id).toBe(id); + + update("Today is Monday."); + expect(field.hasAttribute("aria-describedby")).toBe(false); + }); + + it("gives each mount its own id, so two notes never collide", () => { + const a = host(); + const b = host(); + mountSystemPromptLiteralNote(a.container, a.field, "{{DATE}}"); + mountSystemPromptLiteralNote(b.container, b.field, "{{DATE}}"); + + expect(noteOf(a.container).id).not.toBe(noteOf(b.container).id); + expect(a.field.getAttribute("aria-describedby")).toBe( + noteOf(a.container).id, + ); + expect(b.field.getAttribute("aria-describedby")).toBe( + noteOf(b.container).id, + ); + }); + + it("names the token syntax it is about and points at the prompt template", () => { + const { container, field } = host(); + mountSystemPromptLiteralNote(container, field, "{{DATE}}"); + + const text = noteOf(container).textContent ?? ""; + expect(text).toContain("{{DATE}}"); + expect(text).toContain("prompt template"); + }); + + it("exists in the DOM while hidden, so it renders under its own field", () => { + // Created up front rather than on demand: these modals append as they build, + // so a lazily created note would land at the bottom of the modal. + const { container, field } = host(); + mountSystemPromptLiteralNote(container, field, "no tokens here"); + const trailing = container.createEl("div"); + + const note = noteOf(container); + expect( + field.compareDocumentPosition(note) & Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + expect( + trailing.compareDocumentPosition(note) & Node.DOCUMENT_POSITION_PRECEDING, + ).toBeTruthy(); + }); +}); diff --git a/src/gui/ai/systemPromptLiteralNote.ts b/src/gui/ai/systemPromptLiteralNote.ts new file mode 100644 index 00000000..35921c71 --- /dev/null +++ b/src/gui/ai/systemPromptLiteralNote.ts @@ -0,0 +1,76 @@ +/** + * The AI system prompt is sent to the model VERBATIM. QuickAdd's format syntax + * is applied to the prompt (or prompt template) only: + * + * - `runAIAssistant` formats `targetPrompt`, passes `systemPrompt` raw + * (src/ai/AIAssistant.ts). + * - `Prompt` / `ChunkedPrompt` do the same, and the chunked path even sizes + * its budget with `estimateTokenCount(systemPrompt)` on the raw string. + * - `MacroChoiceEngine.executeAIAssistant` hands `command.systemPrompt` + * straight through while its formatter callback wraps only the prompt. + * - `Agent.buildSeedMessages` pushes `system` raw and formats `prompt`. + * + * All four are pinned by AIAssistant.systemPromptLiteral.test.ts. + * + * Until #1572 decides otherwise, `{{DATE}}` in a system prompt reaches the model + * as the eight characters `{{DATE}}`. The three system-prompt modals used to + * claim the opposite - a live "preview" resolving the tokens, plus a `{{` + * autocomplete offering them (issues #1565, #1568). Both are gone; this note is + * what replaces them. + * + * Deliberately conditional on the value containing `{{`, rather than a sentence + * in `setDesc()` that is always visible. The token-free prose prompt is the + * overwhelming majority - it is the shipped default and the seed for every new + * AI command - and a permanent line about a syntax that does not apply is the + * kind of standing chrome the builders' own hints were designed to avoid + * (#1570). The audience here is narrow and specific: an author who already + * typed a token, most likely on the deleted autocomplete's advice, and who would + * otherwise get no signal at all. + * + * The `{{` trigger over-fires on prose or LaTeX that happens to contain two + * braces (`\frac{{a}}{b}`). That is deliberate: the note is muted and states a + * fact about the field, so a false positive costs one quiet line, while a false + * negative costs a silently broken prompt. + */ + +const LITERAL_NOTE_TEXT = + "QuickAdd sends the system prompt to the model as written - format syntax like {{DATE}} is not resolved here. Only the prompt template is formatted."; + +/** Distinguishes the notes of two modals open at once, and across `reload()`. */ +let noteSequence = 0; + +/** + * Renders a muted note under a system-prompt field, and returns the updater to + * call from the field's `onChange`. + * + * The element is created up front and hidden rather than created on demand: the + * modals build their DOM imperatively and append as they go, so a lazily created + * note would land at the bottom of the modal instead of under its field. + */ +export function mountSystemPromptLiteralNote( + container: HTMLElement, + field: HTMLTextAreaElement, + initialValue: string, +): (value: string) => void { + const note = container.createEl("div", { + text: LITERAL_NOTE_TEXT, + cls: "qa-literal-format-note", + }); + const noteId = `qa-literal-format-note-${++noteSequence}`; + note.id = noteId; + + // classList/setAttribute, not Obsidian's toggleClass: this runs under the + // vitest DOM stub too, which patches createEl/addClass but not toggleClass. + const update = (value: string): void => { + const shown = value.includes("{{"); + note.classList.toggle("qa-literal-format-note--shown", shown); + // Referenced only while shown. An accessible description may include a + // directly-referenced hidden node, so leaving the reference in place would + // describe the field with a note the sighted user cannot see. + if (shown) field.setAttribute("aria-describedby", noteId); + else field.removeAttribute("aria-describedby"); + }; + + update(initialValue); + return update; +} diff --git a/src/styles.css b/src/styles.css index d902de45..16f60860 100644 --- a/src/styles.css +++ b/src/styles.css @@ -651,6 +651,15 @@ box-sizing: border-box; } +/* A format preview mounted under one of the textareas above (#1565). Those carry + `margin-bottom: 1em` for the gap to the NEXT setting, which would leave the + preview row floating midway between the field it describes and the one below. + Pull it back up so it reads as bound to its own field, and let the host carry + the separation instead. */ +.quickAddModal .qa-format-preview-host { + margin-top: -0.75em; +} + .qa-ai-token-count, .qa-ai-token-note { color: var(--text-muted); @@ -660,6 +669,25 @@ font-size: var(--font-ui-smaller); } +/* "This field is sent as written" note under a system prompt that contains `{{` + (#1565/#1568). Muted and font-ui-smaller to match .qa-ai-token-note directly + above it in the same modals: like that line it states how the field behaves, + rather than flagging the value as wrong - a `{{` a user wants the model to + read literally is a legitimate thing to type here. + + Hidden by default rather than created on demand, so it can be appended in DOM + order under the field it belongs to instead of at the end of the modal. */ +.qa-literal-format-note { + display: none; + margin: -0.5em 0 1em; + color: var(--text-muted); + font-size: var(--font-ui-smaller); +} + +.qa-literal-format-note--shown { + display: block; +} + .qa-command-button-row { display: flex; justify-content: flex-end; diff --git a/tests/obsidian-stub.ts b/tests/obsidian-stub.ts index fe94dd68..6eebe9e6 100644 --- a/tests/obsidian-stub.ts +++ b/tests/obsidian-stub.ts @@ -136,6 +136,52 @@ export class ToggleComponent extends BaseComponent { } } +export class SliderComponent extends BaseComponent { + sliderEl: HTMLInputElement; + + constructor(containerEl: HTMLElement) { + super(); + this.sliderEl = document.createElement("input"); + this.sliderEl.type = "range"; + containerEl.appendChild(this.sliderEl); + } + + setLimits(min: number, max: number, step: number): this { + this.sliderEl.min = String(min); + this.sliderEl.max = String(max); + this.sliderEl.step = String(step); + return this; + } + + setValue(value: number): this { + this.sliderEl.value = String(value); + return this; + } + + getValue(): number { + return Number(this.sliderEl.value); + } + + setInstant(): this { + return this; + } + + setDynamicTooltip(): this { + return this; + } + + setTooltip(): this { + return this; + } + + onChange(cb: (value: number) => void): this { + this.sliderEl.addEventListener("change", () => + cb(Number(this.sliderEl.value)), + ); + return this; + } +} + export class DropdownComponent extends BaseComponent { selectEl: HTMLSelectElement; @@ -321,6 +367,11 @@ export class Setting { return this; } + addSlider(cb: (component: SliderComponent) => any): this { + cb(new SliderComponent(this.controlEl)); + return this; + } + addComponent(cb: (el: HTMLElement) => T): this { const el = document.createElement("div"); this.controlEl.appendChild(el);