Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/src/content/docs/docs/AIAssistant.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
237 changes: 237 additions & 0 deletions src/ai/AIAssistant.systemPromptLiteral.test.ts
Original file line number Diff line number Diff line change
@@ -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}}");
});
});
48 changes: 23 additions & 25 deletions src/gui/AIAssistantSettingsModal.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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 {
Expand Down
19 changes: 15 additions & 4 deletions src/gui/ChoiceBuilder/components/FormatPreviewField.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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 () => {
Expand Down
Loading