Desktop: Fixes #15874: AI chat panel: Fix changing the note body causes a cache miss (#15969)

packages/app-desktop/app.reducer.test.ts

@@ -146,13 +146,14 @@ describe('app.reducer', () => {
 				toolCallId: 'call-1',
 				userDescription: '',
 				isError: false,
+				isEdit: true,
 				content: 'Result',
 			} satisfies ChatToolMessage),
 		});
 
 		expect(state.aiChatMessages).toMatchObject([
 			{ id: 'id-0', role: 'user', text: 'Test', raw: [{ role: 'user' }] },
-			{ id: 'id-1', role: 'assistant', text: 'Testing', editsApplied: 1, editsMissed: 0 },
+			{ id: 'id-1', role: 'assistant', text: 'Testing' },
 		]);
 
 

packages/app-desktop/app.reducer.ts

@@ -13,8 +13,7 @@ export interface AiChatMessage {
 	id: string;
 	role: 'user' | 'assistant' | 'error' | 'separator';
 	text: string;
-	editsApplied?: number;
-	editsMissed?: number;
+	hide?: boolean;
 
 	// The raw message(s) corresponding to this event
 	raw: ChatMessage[];
@@ -317,15 +316,8 @@ export default function(state: AppState, action: any) {
 				state, action.windowId, 'aiChatMessages', messages => {
 					let lastMessage = messages[messages.length - 1];
 					if (lastMessage) {
-						const toolCall = action.toolCall;
-						const error = toolCall.isError;
-						const editsApplied = (lastMessage.editsApplied ?? 0) + (error ? 0 : 1);
-						const editsMissed = (lastMessage.editsMissed ?? 0) + (error ? 1 : 0);
-
 						lastMessage = {
 							...lastMessage,
-							editsApplied,
-							editsMissed,
 							raw: [
 								...lastMessage.raw,
 								action.toolCall,

packages/app-desktop/gui/ChatPanel/ChatMessageItem.tsx

@@ -2,17 +2,45 @@ import * as React from 'react';
 import { _ } from '@joplin/lib/locale';
 import { AiChatMessage } from '../../app.reducer';
 import InlineMarkdownDisplay from '../InlineMarkdownDisplay';
-import { ChatMessage, ChatRole } from '@joplin/lib/services/ai/types';
+import { ChatMessage, ChatRole, ChatToolMessage } from '@joplin/lib/services/ai/types';
 
 
-const editsSummary = (actions: ChatMessage[], applied: number, missed: number) => {
-	if (applied + missed === 0) return '';
-	if (missed === 0 && applied > 1) return _('%d edit(s) applied.', applied);
-	if (missed === 0) {
-		const toolResults = actions.filter(action => action.role === ChatRole.Tool);
-		return toolResults.map(result => result.userDescription).join('\n');
-	}
-	return _('%d edit(s) applied, %d could not be placed automatically.', applied, missed);
+const toolResultSummary = (actions: ChatMessage[]) => {
+	const toolResults = actions.filter(action => action.role === ChatRole.Tool);
+
+	const editSummary = () => {
+		let applied = 0;
+		let missed = 0;
+		let lastEdit: ChatToolMessage|null = null;
+		for (const result of toolResults) {
+			if (!result.isEdit) continue;
+
+			if (result.isError) {
+				missed ++;
+			} else {
+				applied ++;
+				lastEdit = result;
+			}
+		}
+
+		if (applied + missed === 0) return '';
+		if (missed === 0 && applied > 1) return _('%d edit(s) applied.', applied);
+		if (missed === 0 && lastEdit) return lastEdit.userDescription;
+		return _('%d edit(s) applied, %d could not be placed automatically.', applied, missed);
+	};
+
+	return [
+		...toolResults
+			.filter(result => !result.isEdit)
+			.map(result => result.userDescription),
+		editSummary(),
+	]
+		.filter(item => !!item)
+		.join('\n');
+};
+
+const hasToolError = (actions: ChatMessage[]) => {
+	return actions.some(action => action.role === ChatRole.Tool && action.isError);
 };
 
 interface Props {
@@ -27,7 +55,7 @@ const ChatMessageItem: React.FC<Props> = ({ message }) => {
 		return <div className='error'>{message.text}</div>;
 	}
 
-	const summary = message.role === 'assistant' ? editsSummary(message.raw, message.editsApplied ?? 0, message.editsMissed ?? 0) : '';
+	const summary = message.role === 'assistant' ? toolResultSummary(message.raw) : '';
 	// Always show something in the message box:
 	const textContent = !message.text && !summary ? _('(no message)') : message.text;
 
@@ -44,7 +72,7 @@ const ChatMessageItem: React.FC<Props> = ({ message }) => {
 			{content}
 			{summary && (
 				<div className='meta'>
-					{(message.editsMissed ?? 0) > 0
+					{hasToolError(message.raw)
 						? <span className='warning'>{summary}</span>
 						: <span>{summary}</span>}
 				</div>

packages/app-desktop/gui/ChatPanel/ChatPanel.tsx

@@ -142,11 +142,13 @@ const ChatPanel: React.FC<Props> = (props) => {
 		lastNoteIdRef.current = props.noteId;
 		if (prev === null || prev === props.noteId || !props.noteId) return;
 		if (messagesLengthRef.current === 0) return;
+
+		const text = _('— now viewing: %s —', props.noteTitle || _('(untitled)'));
 		appendMessage({
 			id: makeId(),
 			role: 'separator',
-			text: _('— now viewing: %s —', props.noteTitle || _('(untitled)')),
-			raw: [],
+			text,
+			raw: [{ role: ChatRole.User, content: _('Switched notes: %s', text) }],
 		});
 	}, [props.noteId, props.noteTitle, appendMessage]);
 
@@ -188,7 +190,7 @@ const ChatPanel: React.FC<Props> = (props) => {
 		// Captured so we can roll it back on failure — otherwise a retry would
 		// send the prior user turn as history alongside the new prompt.
 		const userTurnId = makeId();
-		let hadSuccessfulResponse = false;
+		let hadVisibleResponse = false;
 		appendMessage({
 			id: userTurnId,
 			role: 'user',
@@ -236,15 +238,16 @@ const ChatPanel: React.FC<Props> = (props) => {
 					// System messages are not shown in the UI
 					if (entry.role === ChatRole.System) continue;
 
-					hadSuccessfulResponse = true;
-
 					if (entry.role === ChatRole.Tool) {
 						addToolResult(entry);
 					} else {
+						hadVisibleResponse ||= !entry.hide;
+
 						appendMessage({
 							id: makeId(),
 							role: entry.role,
 							text: entry.content,
+							hide: entry.hide,
 							raw: [entry],
 						});
 					}
@@ -295,7 +298,7 @@ const ChatPanel: React.FC<Props> = (props) => {
 			logger.warn('Chat failed:', error);
 			if (abortController.signal.aborted) return;
 
-			if (!hadSuccessfulResponse) {
+			if (!hadVisibleResponse) {
 				dispatch({ type: 'AI_CHAT_REMOVE', windowId, id: userTurnId });
 				setInput(text);
 			}
@@ -343,15 +346,17 @@ const ChatPanel: React.FC<Props> = (props) => {
 			return { content, showingMessages: false };
 		}
 
+		const visibleMessages = messages.filter(m => !m.hide);
+
 		const content = <>
 			{props.aiDegraded && <AiDegradedNotice className='degraded-status' />}
 			<div className='messages' aria-live={hasFocus ? 'polite' : undefined}>
-				{messages.length === 0 && (
+				{visibleMessages.length === 0 && (
 					<div className='empty'>
 						{_('Ask about this note, or request changes. Select text in the editor first to scope the request to that selection.')}
 					</div>
 				)}
-				{messages.map(m => <ChatMessageItem key={m.id} message={m}/>)}
+				{visibleMessages.map(m => <ChatMessageItem key={m.id} message={m}/>)}
 				<div ref={messagesEndRef} />
 			</div>
 			<div className='composer'>
@@ -385,7 +390,7 @@ const ChatPanel: React.FC<Props> = (props) => {
 			</div>
 		</>;
 
-		return { content, showingMessages: messages.length > 0 };
+		return { content, showingMessages: visibleMessages.length > 0 };
 	};
 
 	const sendButtonLabel = sending ? _('Stop generating') : _('Send');

packages/lib/services/ai/noteChat.test.ts

@@ -62,14 +62,14 @@ describe('noteChat', () => {
 		expect(prompt).toContain('- [ ]');
 	});
 
-	test('systemPrompt includes full body when no selection', () => {
+	test('systemPrompt should not include body', () => {
 		const prompt = _internal.systemPrompt({
 			title: 'My note',
 			body: 'the whole body',
 			selection: null,
 		});
-		expect(prompt).toContain('the whole body');
-		expect(prompt).toContain('BEGIN NOTE');
+		// Including the body in the prompt would cause a cache invalidation on each note change
+		expect(prompt).not.toContain('the whole body');
 	});
 
 	test.each([
@@ -89,7 +89,7 @@ describe('noteChat', () => {
 				body: 'b',
 				selection: null,
 			},
-			expectedOperations: ['insertBefore', 'insertAfter', 'appendToNote', 'replaceRange'],
+			expectedOperations: ['insertBefore', 'insertAfter', 'appendToNote', 'replaceRange', 'readNote'],
 		},
 		{
 			label: 'offers replaceFencedBlock when Mermaid block present',
@@ -98,7 +98,7 @@ describe('noteChat', () => {
 				body: '```mermaid\ngitGraph\n\tcommit\n```\n',
 				selection: null,
 			},
-			expectedOperations: ['insertBefore', 'insertAfter', 'appendToNote', 'replaceRange', 'replaceFencedBlock'],
+			expectedOperations: ['insertBefore', 'insertAfter', 'appendToNote', 'replaceRange', 'replaceFencedBlock', 'readNote'],
 		},
 	])('toolDefinitions should include the expected operations (case $label)', ({ note, expectedOperations }) => {
 		const editSchemaItems = _internal.toolDefinitions(note);
@@ -342,9 +342,34 @@ describe('noteChat', () => {
 			{ role: ChatRole.System },
 			{ role: ChatRole.User, content: userMessage },
 
+			// Reading the body
+			{ role: ChatRole.Assistant },
+			{ role: ChatRole.Tool },
+
 			...failedAttempts,
 
 			{ role: ChatRole.Assistant, content: 'tool not found: replaceRange\ntool not found: appendToNote' },
 		]);
 	});
+
+	test('noteChat initialize the chat history with the note body', async () => {
+		const { onContext, commands } = makeTestContext();
+
+		const result = await runNoteChat(
+			onContext,
+			[],
+			'test',
+			commands,
+			() => {},
+			new AbortController().signal,
+		);
+
+		expect(result).toMatchObject([
+			{ role: ChatRole.System },
+			{ role: ChatRole.User, content: 'test' },
+			{ role: ChatRole.Assistant, content: '', toolCalls: [{ toolName: 'readNote' }] },
+			{ role: ChatRole.Tool, content: 'Body', toolName: 'readNote' },
+			{ role: ChatRole.Assistant, content: '' },
+		]);
+	});
 });

packages/lib/services/ai/noteChat.ts

@@ -60,6 +60,8 @@ const hasStructuredBlock = (note: NoteContext) => {
 	return supportedStructuredBlockTags.some(tag => !!findFencedBlock(note.body, tag, 0));
 };
 
+// The system prompt should be relatively constant across note changes to take advantage of prompt caching.
+// See https://developers.openai.com/api/docs/guides/prompt-caching
 const systemPrompt = (note: NoteContext) => {
 	const lines: string[] = [
 		'You are an assistant helping the user work on a note in Joplin, a note-taking application.',
@@ -76,12 +78,6 @@ const systemPrompt = (note: NoteContext) => {
 		lines.push(note.selection);
 		lines.push('--- END SELECTION ---');
 		lines.push();
-	} else {
-		lines.push('Note body:');
-		lines.push('--- BEGIN NOTE ---');
-		lines.push(note.body);
-		lines.push('--- END NOTE ---');
-		lines.push();
 	}
 
 	if (note.selection) {
@@ -116,6 +112,18 @@ const toolDefinitions = (note: NoteContext) => {
 			},
 		});
 	} else {
+		result.push(
+			{
+				name: 'readNote',
+				description: 'Get the current, up-to-date content of the note.',
+				inputSchema: {
+					type: 'object',
+					required: [],
+					additionalProperties: false,
+				},
+			},
+		);
+
 		result.push(
 			{
 				name: 'appendToNote',
@@ -199,6 +207,45 @@ const toolDefinitions = (note: NoteContext) => {
 	return result;
 };
 
+const createHistory = (history: ChatMessage[], newMessage: string, context: NoteContext): ChatMessage[] => {
+	history = removeSystemPrompt(history);
+
+	const isNewChat = history.length === 0 && !context.selection;
+	history = [
+		{ role: ChatRole.System, content: systemPrompt(context) },
+		...history,
+		{ role: ChatRole.User, content: newMessage },
+	];
+
+	// The assistant almost always needs to fetch the current content of the note. Initializing the
+	// chat transcript with a `readNote` tool call saves one round-trip:
+	const addReadNote = isNewChat;
+	if (addReadNote) {
+		const callId = `call_read${history.length}`;
+		history.push(
+			{
+				role: ChatRole.Assistant,
+				content: '',
+				hide: true,
+				toolCalls: [
+					{ callId, arguments: { }, toolName: 'readNote', parseError: null },
+				],
+			},
+			{
+				role: ChatRole.Tool,
+				content: context.body,
+				toolCallId: callId,
+				toolName: 'readNote',
+				userDescription: '',
+				isEdit: false,
+				isError: false,
+			},
+		);
+	}
+
+	return history;
+};
+
 const estimateTokens = (text: string) => Math.ceil(text.length / charsPerToken);
 
 export interface ChatCommands {
@@ -218,14 +265,8 @@ export const runNoteChat = async (
 	onHistoryChanged: OnHistoryChanged,
 	signal: AbortSignal,
 ) => {
-	history = [
-		// Ensure that the system prompt is up-to-date with the context.
-		// TODO: This will invalidate the prompt cache.
-		//       See https://developers.openai.com/api/docs/guides/prompt-caching
-		{ role: ChatRole.System, content: systemPrompt(await context()) },
-		...removeSystemPrompt(history),
-		{ role: ChatRole.User, content: userMessage },
-	];
+	const initialContext = await context();
+	history = createHistory(history, userMessage, initialContext);
 	onHistoryChanged([...history]);
 
 	const hasUndeliveredToolResults = () => history[history.length - 1]?.role === ChatRole.Tool;
@@ -343,14 +384,17 @@ const runTools = async (chat: ChatResult, initialContext: NoteContext, context:
 
 	let chatResponses: ChatToolMessage[] = [];
 
-	const respondSuccess = (action: ChatToolCall, message: string) => {
+	const isEdit = (toolName: string) => isValidEditOp(toolName);
+
+	const respondSuccess = (action: ChatToolCall, message: string, userDescription?: string) => {
 		chatResponses.push({
 			role: ChatRole.Tool,
 			toolName: action.toolName,
 			toolCallId: action.callId,
 			content: message,
-			userDescription: message,
+			userDescription: userDescription ?? message,
 			isError: false,
+			isEdit: isEdit(action.toolName),
 		});
 	};
 
@@ -362,6 +406,7 @@ const runTools = async (chat: ChatResult, initialContext: NoteContext, context:
 			content: `failed: ${reason}`,
 			userDescription: reason,
 			isError: true,
+			isEdit: isEdit(action.toolName),
 		});
 	};
 
@@ -400,47 +445,45 @@ const runTools = async (chat: ChatResult, initialContext: NoteContext, context:
 	} else {
 		const initialBody = initialContext.body;
 		let body = currentContext.body;
-
-		if (initialBody !== body && chat.toolCalls.length > 0) {
-			for (const toolCall of chat.toolCalls) {
-				respondFailure(toolCall, 'body changed externally, before edit could be applied');
-			}
-			commands.displayError(_('The note changed while the request was running; edits were not applied. Try again.'));
-
-			return chatResponses;
-		}
+		const hadExternalBodyChanges = initialBody !== body;
+		let attemptedEdit = false;
 
 		for (const toolCall of chat.toolCalls) {
 			if (toolCall.toolName === 'replaceSelection') {
 				respondFailure(toolCall, 'replaceSelection is invalid in this context');
-				continue;
-			}
-			if (!isValidEditOp(toolCall.toolName)) {
-				respondFailure(toolCall, 'tool not found');
-				continue;
-			}
-			if (toolCall.parseError) {
+			} else if (toolCall.parseError) {
 				respondFailure(toolCall, toolCall.parseError);
-				continue;
-			}
-
-			const editOperation = toolCallToEditOperation(toolCall);
-			const { newBody, appliedEdits } = applyAnchorEdits(body, [editOperation], 0);
-			body = newBody;
-
-			if (appliedEdits.length !== 1) {
-				throw new Error(`Invalid state: Wrong number of applied edits, ${appliedEdits.length}`);
-			}
-
-			const edit = appliedEdits[0];
-			if (edit.status === 'applied') {
-				respondSuccess(toolCall, describeEditOperation(editOperation));
+			} else if (toolCall.toolName === 'readNote') {
+				respondSuccess(toolCall, body, _('Read note'));
+			} else if (!isValidEditOp(toolCall.toolName)) {
+				respondFailure(toolCall, 'tool not found');
+			} else if (hadExternalBodyChanges) {
+				attemptedEdit = true;
+				respondFailure(toolCall, 'body changed externally, before edit could be applied');
 			} else {
-				respondFailure(toolCall, edit.status);
+				attemptedEdit = true;
+				const editOperation = toolCallToEditOperation(toolCall);
+				const { newBody, appliedEdits } = applyAnchorEdits(body, [editOperation], 0);
+				body = newBody;
+
+				if (appliedEdits.length !== 1) {
+					throw new Error(`Invalid state: Wrong number of applied edits, ${appliedEdits.length}`);
+				}
+
+				const edit = appliedEdits[0];
+				if (edit.status === 'applied') {
+					respondSuccess(toolCall, describeEditOperation(editOperation));
+				} else {
+					respondFailure(toolCall, edit.status);
+				}
 			}
 		}
 
-		if (body !== initialBody) {
+		if (attemptedEdit && hadExternalBodyChanges) {
+			commands.displayError(_('The note changed while the request was running; edits were not applied. Try again.'));
+		}
+		// Avoid overwriting user-created changes
+		if (!hadExternalBodyChanges && body !== initialBody) {
 			try {
 				await commands.updateNoteBody(body, initialBody);
 			} catch (error) {

packages/lib/services/ai/providers/Anthropic.test.ts

@@ -20,6 +20,7 @@ describe('ai/providers/Anthropic', () => {
 				toolCallId: 'call-1',
 				toolName: 'replaceRange',
 				isError: false,
+				isEdit: true,
 			},
 			{
 				role: ChatRole.Tool,
@@ -28,6 +29,7 @@ describe('ai/providers/Anthropic', () => {
 				toolCallId: 'call-2',
 				toolName: 'replaceRange',
 				isError: true,
+				isEdit: true,
 			},
 			{
 				role: ChatRole.User,

packages/lib/services/ai/types.ts

@@ -11,6 +11,7 @@ interface ChatBaseMessage {
 
 export interface ChatStandardMessage extends ChatBaseMessage {
 	role: ChatRole.System | ChatRole.User | ChatRole.Assistant;
+	hide?: boolean;
 	toolCalls?: ChatToolCall[];
 }
 
@@ -21,6 +22,7 @@ export interface ChatToolMessage extends ChatBaseMessage {
 	isError: boolean;
 	// A very brief description of the result that can be shown to the user
 	userDescription: string;
+	isEdit: boolean;
 }
 
 export type ChatMessage = ChatStandardMessage | ChatToolMessage;