diff --git a/AIClasses/Claude/Claude.ts b/AIClasses/Claude/Claude.ts index a091895..ef6ae84 100644 --- a/AIClasses/Claude/Claude.ts +++ b/AIClasses/Claude/Claude.ts @@ -228,7 +228,8 @@ export class Claude implements IAIClass { role: content.role, content: contentBlocks }; - }); + }) + .filter(message => message.content.length > 0); } private mapFunctionDefinitions(aiFunctionDefinitions: IAIFunctionDefinition[]): object[] { diff --git a/AIClasses/Gemini/Gemini.ts b/AIClasses/Gemini/Gemini.ts index 49c1ac0..905160d 100644 --- a/AIClasses/Gemini/Gemini.ts +++ b/AIClasses/Gemini/Gemini.ts @@ -210,9 +210,10 @@ export class Gemini implements IAIClass { return { role: content.role === Role.User ? Role.User : Role.Model, - parts: parts.length > 0 ? parts : [{ text: "" }] + parts: parts }; - }); + }) + .filter(message => message.parts.length > 0); } private mapFunctionDefinitions(aiFunctionDefinitions: IAIFunctionDefinition[]): object[] { diff --git a/AIClasses/OpenAI/OpenAI.ts b/AIClasses/OpenAI/OpenAI.ts index 03d6864..16052c3 100644 --- a/AIClasses/OpenAI/OpenAI.ts +++ b/AIClasses/OpenAI/OpenAI.ts @@ -126,6 +126,7 @@ export class OpenAI implements IAIClass { content: contentToExtract }; }) + .filter(message => message.content !== "" || message.tool_calls || message.tool_call_id) ]; const tools = this.mapFunctionDefinitions( diff --git a/Components/ChatArea.svelte b/Components/ChatArea.svelte index 4331616..8bdf10a 100644 --- a/Components/ChatArea.svelte +++ b/Components/ChatArea.svelte @@ -175,13 +175,17 @@
{#each messages as message, index} - {#if !message.isFunctionCallResponse && message.content} -
-
- {#if message.role === Role.User} + {#if !message.isFunctionCallResponse && message.content.trim() !== ""} + {#if message.role === Role.User} +
+
- {:else} - {@const messageId = message.timestamp.getTime().toString()} +
+
+ {:else} + {@const messageId = message.timestamp.getTime().toString()} +
+
{#if currentStreamingMessageId === messageId}
@@ -189,9 +193,9 @@ {@html getStaticHTML(message)} {/if}
- {/if} +
-
+ {/if} {/if} {/each} diff --git a/Enums/Copy.ts b/Enums/Copy.ts index 7eb2a03..33114cb 100644 --- a/Enums/Copy.ts +++ b/Enums/Copy.ts @@ -58,7 +58,9 @@ export enum Copy { // Help Modal Copy HelpModalAboutTitle = "About", - HelpModalAboutContent = "", + HelpModalAboutContent = `### About AI Agent + +**AI Agent** brings the power of Claude, Gemini, and OpenAI directly into your Obsidian vault with intelligent note management capabilities.`, HelpModalGuideTitle = "Plugin Guide", HelpModalGuideContent = "", @@ -69,6 +71,9 @@ export enum Copy { HelpModalPrivacyTitle = "Privacy", HelpModalPrivacyContent = "", + // SVG Icons + GitHubIconPath = "M48.854 0C21.839 0 0 22 0 49.217c0 21.756 13.993 40.172 33.405 46.69 2.427.49 3.316-1.059 3.316-2.362 0-1.141-.08-5.052-.08-9.127-13.59 2.934-16.42-5.867-16.42-5.867-2.184-5.704-5.42-7.17-5.42-7.17-4.448-3.015.324-3.015.324-3.015 4.934.326 7.523 5.052 7.523 5.052 4.367 7.496 11.404 5.378 14.235 4.074.404-3.178 1.699-5.378 3.074-6.6-10.839-1.141-22.243-5.378-22.243-24.283 0-5.378 1.94-9.778 5.014-13.2-.485-1.222-2.184-6.275.486-13.038 0 0 4.125-1.304 13.426 5.052a46.97 46.97 0 0 1 12.214-1.63c4.125 0 8.33.571 12.213 1.63 9.302-6.356 13.427-5.052 13.427-5.052 2.67 6.763.97 11.816.485 13.038 3.155 3.422 5.015 7.822 5.015 13.2 0 18.905-11.404 23.06-22.324 24.283 1.78 1.548 3.316 4.481 3.316 9.126 0 6.6-.08 11.897-.08 13.526 0 1.304.89 2.853 3.316 2.364 19.412-6.52 33.405-24.935 33.405-46.691C97.707 22 75.788 0 48.854 0z", + // Example Content EXAMPLE_USER_INSTRUCTION = `### TL;DR diff --git a/Modals/HelpModalSvelte.svelte b/Modals/HelpModalSvelte.svelte index 72105a9..920e72f 100644 --- a/Modals/HelpModalSvelte.svelte +++ b/Modals/HelpModalSvelte.svelte @@ -148,6 +148,36 @@ {#if content !== ""}
{@html content} + {#if selectedTopic === 1} +
+

Links

+ +
+

Created with ❤️ for the Obsidian community

+ {/if}
{/if}
diff --git a/Services/ChatService.ts b/Services/ChatService.ts index 422f380..fb09487 100644 --- a/Services/ChatService.ts +++ b/Services/ChatService.ts @@ -90,6 +90,7 @@ export class ChatService { )); } + this.ensureCorrectConversationStructure(conversation); response = await this.streamRequestResponse(conversation, allowDestructiveActions, callbacks); } } finally { @@ -141,6 +142,26 @@ export class ChatService { this.statusBarService.animateTokens(inputTokens, outputTokens); } + private ensureCorrectConversationStructure(conversation: Conversation) { + // Check if the last message is from the assistant to prevent assistant-to-assistant structure + // This can happen when the assistant's last message had no function call and the user sends a new request + if (conversation.contents.length > 0) { + const lastMessage = conversation.contents[conversation.contents.length - 1]; + if (lastMessage.role === Role.Assistant) { + // Insert a hidden "Continue" message to maintain proper conversation structure + conversation.contents.push(new ConversationContent( + Role.User, + "Continue", + "Continue", + "", + new Date(), + false, + true // isFunctionCallResponse = true (hides from UI) + )); + } + } + } + private async streamRequestResponse( conversation: Conversation, allowDestructiveActions: boolean, callbacks: IChatServiceCallbacks ): Promise<{ functionCall: AIFunctionCall | null, shouldContinue: boolean }> { @@ -175,18 +196,21 @@ export class ChatService { if (chunk.content) { accumulatedContent += chunk.content; conversation.setMostRecentContent(accumulatedContent); - callbacks.onThoughtUpdate(null); + if (accumulatedContent.trim() !== "") { + callbacks.onThoughtUpdate(null); + } } if (chunk.isComplete) { - // No content and no function call - remove empty message - if (accumulatedContent.trim() == "" && !capturedFunctionCall) { - conversation.contents.pop(); - } + const sanitizedContent = this.sanitizeFunctionCallContent(accumulatedContent, capturedFunctionCall); - conversation.setMostRecentContent(accumulatedContent); - if (capturedFunctionCall) { - conversation.setMostRecentFunctionCall(capturedFunctionCall?.toConversationString()); + if (sanitizedContent.trim() === "" && !capturedFunctionCall) { + conversation.contents.pop(); + } else { + conversation.setMostRecentContent(sanitizedContent); + if (capturedFunctionCall) { + conversation.setMostRecentFunctionCall(capturedFunctionCall?.toConversationString()); + } } } callbacks.onStreamingUpdate(aiMessage.timestamp.getTime().toString()); @@ -196,4 +220,63 @@ export class ChatService { return { functionCall: capturedFunctionCall, shouldContinue: capturedShouldContinue }; } + + // handle the rare event where a function call is also included in content (gemini sometimes does this) + private sanitizeFunctionCallContent(content: string, functionCall: AIFunctionCall | null): string { + // Early returns for simple cases + if (!functionCall || !content.trim()) { + return content; + } + + // If content has no JSON-like characters, return as-is + if (!content.includes('{') || !content.includes('}')) { + return content; + } + + const functionCallString = functionCall.toConversationString(); + let sanitized = content; + + // Step 1: Remove markdown code blocks that might contain the function call + // Pattern matches ```json\n...\n``` or ```\n...\n``` + sanitized = sanitized.replace(/```(?:json)?\s*\n?([\s\S]*?)\n?```/g, (match, codeContent) => { + // If the code block contains our function call, remove it entirely + if (codeContent.trim() === functionCallString.trim()) { + return ''; + } + // Otherwise keep the code block + return match; + }); + + // Step 2: Remove exact JSON match (handles compact JSON) + sanitized = sanitized.replace(functionCallString, '').trim(); + + // Step 3: Handle pretty-printed variations by normalizing both strings + try { + const functionCallObj = JSON.parse(functionCallString); + const normalizedTarget = JSON.stringify(functionCallObj); + + // Find and remove any JSON that matches when normalized + // This regex finds JSON objects/arrays in the text + const jsonPattern = /\{(?:[^{}]|(?:\{(?:[^{}]|(?:\{[^{}]*\}))*\}))*\}|\[(?:[^\[\]]|(?:\[(?:[^\[\]]|(?:\[[^\[\]]*\]))*\]))*\]/g; + + sanitized = sanitized.replace(jsonPattern, (match) => { + try { + const parsedMatch = JSON.parse(match); + const normalizedMatch = JSON.stringify(parsedMatch); + // Remove if it matches our function call when normalized + return normalizedMatch === normalizedTarget ? '' : match; + } catch { + // If it's not valid JSON, keep it + return match; + } + }); + } catch { + // If function call string isn't valid JSON, we've done what we can + } + + // Step 4: Clean up multiple consecutive whitespace/newlines left by removals + sanitized = sanitized.replace(/\n{3,}/g, '\n\n').trim(); + + return sanitized; + } } diff --git a/__tests__/AIClasses/Gemini.test.ts b/__tests__/AIClasses/Gemini.test.ts index eabfc94..b0a2e73 100644 --- a/__tests__/AIClasses/Gemini.test.ts +++ b/__tests__/AIClasses/Gemini.test.ts @@ -310,7 +310,7 @@ describe('Gemini', () => { describe('Message Format Conversion', () => { it('should convert roles to User/Model', async () => { const conversation = new Conversation(); - conversation.contents.push(new ConversationContent(Role.User, 'Hello')); + conversation.contents.push(new ConversationContent(Role.User, 'Hello', 'Hello')); conversation.contents.push(new ConversationContent(Role.Assistant, 'Hi there')); mockStreamingService.streamRequest.mockImplementation(async function* () { @@ -416,9 +416,8 @@ describe('Gemini', () => { const result = (gemini as any).extractContents([invalidContent]); - // Should result in empty parts (no text, no function call) - expect(result).toHaveLength(1); - expect(result[0].parts).toEqual([{ text: '' }]); + // Should be filtered out since it has no valid parts (no text, invalid function call) + expect(result).toHaveLength(0); expect(consoleSpy).toHaveBeenCalled(); consoleSpy.mockRestore(); diff --git a/__tests__/Services/ChatService.test.ts b/__tests__/Services/ChatService.test.ts index 7ad0602..c5ed6fe 100644 --- a/__tests__/Services/ChatService.test.ts +++ b/__tests__/Services/ChatService.test.ts @@ -5,6 +5,7 @@ import { Services } from '../../Services/Services'; import { Conversation } from '../../Conversations/Conversation'; import { ConversationContent } from '../../Conversations/ConversationContent'; import { Role } from '../../Enums/Role'; +import { AIFunctionCall } from '../../AIClasses/AIFunctionCall'; /** * INTEGRATION TESTS - Simplified @@ -207,4 +208,423 @@ describe('ChatService - Integration Tests (Sync Methods Only)', () => { expect(newService.onNameChanged).toBeUndefined(); }); }); + + describe('assistant-to-assistant message prevention', () => { + it('should insert Continue message when last message is from assistant', () => { + const conversation = new Conversation(); + // Add an initial user message + conversation.contents.push(new ConversationContent(Role.User, 'First message')); + // Add an assistant response + conversation.contents.push(new ConversationContent(Role.Assistant, 'Assistant response')); + + // Verify the last message is from assistant + expect(conversation.contents[conversation.contents.length - 1].role).toBe(Role.Assistant); + expect(conversation.contents.length).toBe(2); + + // Now we need to simulate what happens in submit() when a new user message is added + // Check if the last message is from the assistant (like the fix does) + if (conversation.contents.length > 0) { + const lastMessage = conversation.contents[conversation.contents.length - 1]; + if (lastMessage.role === Role.Assistant) { + conversation.contents.push(new ConversationContent( + Role.User, + "Continue", + "Continue", + "", + new Date(), + false, + true + )); + } + } + + // Verify Continue message was inserted + expect(conversation.contents.length).toBe(3); + const continueMessage = conversation.contents[2]; + expect(continueMessage.role).toBe(Role.User); + expect(continueMessage.content).toBe('Continue'); + expect(continueMessage.isFunctionCallResponse).toBe(true); + }); + + it('should NOT insert Continue message when last message is from user', () => { + const conversation = new Conversation(); + // Add a user message + conversation.contents.push(new ConversationContent(Role.User, 'User message')); + + // Verify the last message is from user + expect(conversation.contents[conversation.contents.length - 1].role).toBe(Role.User); + expect(conversation.contents.length).toBe(1); + + // Simulate the check from submit() + if (conversation.contents.length > 0) { + const lastMessage = conversation.contents[conversation.contents.length - 1]; + if (lastMessage.role === Role.Assistant) { + conversation.contents.push(new ConversationContent( + Role.User, + "Continue", + "Continue", + "", + new Date(), + false, + true + )); + } + } + + // Verify NO Continue message was inserted + expect(conversation.contents.length).toBe(1); + }); + + it('should NOT insert Continue message for empty conversation', () => { + const conversation = new Conversation(); + + // Verify empty + expect(conversation.contents.length).toBe(0); + + // Simulate the check from submit() + if (conversation.contents.length > 0) { + const lastMessage = conversation.contents[conversation.contents.length - 1]; + if (lastMessage.role === Role.Assistant) { + conversation.contents.push(new ConversationContent( + Role.User, + "Continue", + "Continue", + "", + new Date(), + false, + true + )); + } + } + + // Verify still empty + expect(conversation.contents.length).toBe(0); + }); + + it('should maintain proper structure after Continue message insertion', () => { + const conversation = new Conversation(); + conversation.contents.push(new ConversationContent(Role.User, 'First')); + conversation.contents.push(new ConversationContent(Role.Assistant, 'Response')); + + // Simulate the fix + if (conversation.contents.length > 0) { + const lastMessage = conversation.contents[conversation.contents.length - 1]; + if (lastMessage.role === Role.Assistant) { + conversation.contents.push(new ConversationContent( + Role.User, + "Continue", + "Continue", + "", + new Date(), + false, + true + )); + } + } + + // Add the actual user message + conversation.contents.push(new ConversationContent(Role.User, 'Second message')); + + // Verify the structure: User -> Assistant -> User(Continue) -> User + expect(conversation.contents.length).toBe(4); + expect(conversation.contents[0].role).toBe(Role.User); + expect(conversation.contents[0].content).toBe('First'); + expect(conversation.contents[1].role).toBe(Role.Assistant); + expect(conversation.contents[1].content).toBe('Response'); + expect(conversation.contents[2].role).toBe(Role.User); + expect(conversation.contents[2].content).toBe('Continue'); + expect(conversation.contents[2].isFunctionCallResponse).toBe(true); + expect(conversation.contents[3].role).toBe(Role.User); + expect(conversation.contents[3].content).toBe('Second message'); + }); + + it('should work with function call responses in conversation', () => { + const conversation = new Conversation(); + conversation.contents.push(new ConversationContent(Role.User, 'Request')); + conversation.contents.push(new ConversationContent(Role.Assistant, 'Making a function call', '', '{"name":"test"}', new Date(), true)); + // Function response (already User role with isFunctionCallResponse) + conversation.contents.push(new ConversationContent(Role.User, 'Function result', 'Function result', '', new Date(), false, true, 'tool-1')); + // Assistant processes the function response + conversation.contents.push(new ConversationContent(Role.Assistant, 'Final response')); + + // Now the last message is Assistant, so Continue should be inserted + expect(conversation.contents[conversation.contents.length - 1].role).toBe(Role.Assistant); + + // Simulate the fix + if (conversation.contents.length > 0) { + const lastMessage = conversation.contents[conversation.contents.length - 1]; + if (lastMessage.role === Role.Assistant) { + conversation.contents.push(new ConversationContent( + Role.User, + "Continue", + "Continue", + "", + new Date(), + false, + true + )); + } + } + + // Verify Continue was inserted + expect(conversation.contents.length).toBe(5); + expect(conversation.contents[4].content).toBe('Continue'); + expect(conversation.contents[4].isFunctionCallResponse).toBe(true); + }); + }); + + describe('sanitizeFunctionCallContent (private method tests via reflection)', () => { + // Access private method for testing + const getSanitizeMethod = (service: ChatService) => { + return (service as any).sanitizeFunctionCallContent.bind(service); + }; + + it('should return content unchanged when no function call is provided', () => { + const sanitize = getSanitizeMethod(service); + const content = 'This is regular content with {"some": "json"}'; + + const result = sanitize(content, null); + + expect(result).toBe(content); + }); + + it('should return content unchanged when content is empty', () => { + const sanitize = getSanitizeMethod(service); + const functionCall = new AIFunctionCall('test_function', { arg1: 'value1' }); + + const result = sanitize('', functionCall); + + expect(result).toBe(''); + }); + + it('should return content unchanged when content is only whitespace', () => { + const sanitize = getSanitizeMethod(service); + const functionCall = new AIFunctionCall('test_function', { arg1: 'value1' }); + + const result = sanitize(' \n\t ', functionCall); + + expect(result).toBe(' \n\t '); + }); + + it('should remove exact function call JSON from content', () => { + const sanitize = getSanitizeMethod(service); + const functionCall = new AIFunctionCall('write_file', { + file_path: 'test.md', + content: 'Hello world' + }); + const functionCallString = functionCall.toConversationString(); + const content = `Here is the function call: ${functionCallString}`; + + const result = sanitize(content, functionCall); + + expect(result).toBe('Here is the function call:'); + }); + + it('should remove pretty-printed function call JSON (2 spaces)', () => { + const sanitize = getSanitizeMethod(service); + const functionCall = new AIFunctionCall('search_files', { + query: 'test query', + limit: 10 + }); + + const prettyPrinted = JSON.stringify(JSON.parse(functionCall.toConversationString()), null, 2); + const content = `Function call:\n${prettyPrinted}\nEnd of call`; + + const result = sanitize(content, functionCall); + + expect(result).toBe('Function call:\n\nEnd of call'); + }); + + it('should remove pretty-printed function call JSON (4 spaces)', () => { + const sanitize = getSanitizeMethod(service); + const functionCall = new AIFunctionCall('read_file', { + file_path: 'example.ts' + }); + + const prettyPrinted = JSON.stringify(JSON.parse(functionCall.toConversationString()), null, 4); + const content = prettyPrinted; + + const result = sanitize(content, functionCall); + + expect(result).toBe(''); + }); + + it('should preserve legitimate JSON content when no function call exists', () => { + const sanitize = getSanitizeMethod(service); + const legitimateJson = JSON.stringify({ + user: 'test', + data: { nested: 'value' } + }, null, 2); + const content = `Here is your data:\n${legitimateJson}`; + + const result = sanitize(content, null); + + expect(result).toBe(content); + }); + + it('should only remove function call JSON, preserving other content', () => { + const sanitize = getSanitizeMethod(service); + const functionCall = new AIFunctionCall('delete_file', { + file_path: 'old.txt' + }); + const functionCallString = functionCall.toConversationString(); + const content = `Processing your request...\n${functionCallString}\nOperation completed successfully.`; + + const result = sanitize(content, functionCall); + + expect(result).toBe('Processing your request...\n\nOperation completed successfully.'); + }); + + it('should handle function calls with special characters in arguments', () => { + const sanitize = getSanitizeMethod(service); + const functionCall = new AIFunctionCall('write_file', { + file_path: 'path/to/file.ts', + content: 'const regex = /test.*pattern/g;\nfunction() { return "value"; }' + }); + const functionCallString = functionCall.toConversationString(); + const content = functionCallString; + + const result = sanitize(content, functionCall); + + expect(result).toBe(''); + }); + + it('should handle function calls with nested objects', () => { + const sanitize = getSanitizeMethod(service); + const functionCall = new AIFunctionCall('complex_function', { + config: { + nested: { + deeply: { + value: 'test' + } + } + }, + array: [1, 2, 3] + }); + const functionCallString = functionCall.toConversationString(); + const content = `Before\n${functionCallString}\nAfter`; + + const result = sanitize(content, functionCall); + + expect(result).toBe('Before\n\nAfter'); + }); + + it('should handle function calls with toolId', () => { + const sanitize = getSanitizeMethod(service); + const functionCall = new AIFunctionCall('test_function', { arg: 'value' }, 'tool-123'); + const functionCallString = functionCall.toConversationString(); + const content = functionCallString; + + const result = sanitize(content, functionCall); + + expect(result).toBe(''); + }); + + it('should not remove similar but different JSON structures', () => { + const sanitize = getSanitizeMethod(service); + const functionCall = new AIFunctionCall('write_file', { + file_path: 'test.md' + }); + + // Similar but different structure - should not be removed + const differentJson = JSON.stringify({ + functionCall: { + name: 'write_file', + args: { file_path: 'different.md' } + } + }); + const content = `Here is a different call: ${differentJson}`; + + const result = sanitize(content, functionCall); + + // Should keep the different JSON since it doesn't match exactly + expect(result).toContain('different.md'); + }); + + it('should handle multiple whitespace variations in pretty-printed JSON', () => { + const sanitize = getSanitizeMethod(service); + const functionCall = new AIFunctionCall('test', { key: 'value' }); + + // Content with irregular whitespace + const content = `{ + "functionCall": { + "name": "test", + "args": { + "key": "value" + } + } +}`; + + const result = sanitize(content, functionCall); + + expect(result).toBe(''); + }); + + it('should preserve natural language content when function call exists', () => { + const sanitize = getSanitizeMethod(service); + const functionCall = new AIFunctionCall('write_vault_file', { + content: 'Character sheet content', + file_name: 'Walker.md' + }); + const content = 'I have created a note for Omaz and will now create a note for walker'; + + const result = sanitize(content, functionCall); + + // Natural language content should be preserved - it doesn't look like JSON + expect(result).toBe(content); + }); + + it('should preserve content that does not start with {', () => { + const sanitize = getSanitizeMethod(service); + const functionCall = new AIFunctionCall('search_vault_files', { + search_terms: ['Walker'] + }); + const content = 'Searching for details about Walker in Campaign 2 folder.'; + + const result = sanitize(content, functionCall); + + // Content doesn't look like JSON, should be preserved + expect(result).toBe(content); + }); + + it('should preserve content that does not end with }', () => { + const sanitize = getSanitizeMethod(service); + const functionCall = new AIFunctionCall('test', { key: 'value' }); + const content = '{ "incomplete": "json"'; + + const result = sanitize(content, functionCall); + + // Malformed JSON should be preserved + expect(result).toBe(content); + }); + + it('should preserve content with mixed text and JSON-like structures', () => { + const sanitize = getSanitizeMethod(service); + const functionCall = new AIFunctionCall('test', { key: 'value' }); + const content = 'Here is some info: { "data": "example" } and more text'; + + const result = sanitize(content, functionCall); + + // Mixed content should be preserved - doesn't match the exact JSON pattern + expect(result).toBe(content); + }); + + it('should only sanitize when content is EXACTLY matching JSON structure', () => { + const sanitize = getSanitizeMethod(service); + const functionCall = new AIFunctionCall('write_file', { + file_path: 'test.md', + content: 'Hello' + }); + const functionCallString = functionCall.toConversationString(); + + // Only exact matches should be removed + const exactMatch = functionCallString; + const withPrefix = `Creating file: ${functionCallString}`; + const withSuffix = `${functionCallString} - done`; + + expect(sanitize(exactMatch, functionCall)).toBe(''); + expect(sanitize(withPrefix, functionCall)).toBe('Creating file:'); + expect(sanitize(withSuffix, functionCall)).toBe('- done'); + }); + }); });