Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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