fix: prevent empty messages and assistant-to-assistant structure

- Filter out messages with empty content in Claude, Gemini, and OpenAI
- Insert hidden "Continue" message when last message is from assistant
- Remove function call JSON from assistant message content
- Hide empty messages in ChatArea UI
- Add GitHub link and about content to help modal
- Add comprehensive tests for message handling edge cases
This commit is contained in:
Andrew Beal 2025-11-09 13:57:47 +00:00
parent e3c0728c24
commit 0812e91c37
9 changed files with 568 additions and 24 deletions

View file

@ -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[] {

View file

@ -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[] {

View file

@ -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(

View file

@ -175,13 +175,17 @@
<div class="chat-area" bind:this={chatContainer}>
{#each messages as message, index}
{#if !message.isFunctionCallResponse && message.content}
<div class="message-container {message.role === Role.User ? Role.User : Role.Assistant}" use:trackingAction={index}>
<div class="message-bubble {message.role === Role.User ? Role.User : Role.Assistant}">
{#if message.role === Role.User}
{#if !message.isFunctionCallResponse && message.content.trim() !== ""}
{#if message.role === Role.User}
<div class="message-container {Role.User}" use:trackingAction={index}>
<div class="message-bubble {Role.User}">
<div class="message-text-user fade-in-fast" contenteditable="false" bind:innerHTML={message.content}></div>
{:else}
{@const messageId = message.timestamp.getTime().toString()}
</div>
</div>
{:else}
{@const messageId = message.timestamp.getTime().toString()}
<div class="message-container {Role.Assistant}" use:trackingAction={index}>
<div class="message-bubble {Role.Assistant}">
<div class="markdown-content fade-in-fast {currentStreamingMessageId === messageId ? "streaming" : ""}">
{#if currentStreamingMessageId === messageId}
<div use:streamingAction={messageId} class="streaming-content"></div>
@ -189,9 +193,9 @@
{@html getStaticHTML(message)}
{/if}
</div>
{/if}
</div>
</div>
</div>
{/if}
{/if}
{/each}

View file

@ -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

View file

@ -148,6 +148,36 @@
{#if content !== ""}
<div transition:fade={{ duration: 100 }}>
{@html content}
{#if selectedTopic === 1}
<hr style="margin: 2em 0; border-width: 1px">
<h4>Links</h4>
<ul style="list-style: none; padding-left: 0;">
<li style="margin-bottom: 0.5em;">
<a
href="https://github.com/yourusername/ai-agent-plugin"
style="text-decoration: none; display: inline-flex; align-items: center; gap: 0.5em;">
<svg
width="1em"
height="1em"
viewBox="0 0 98 96"
xmlns="http://www.w3.org/2000/svg"
aria-label="GitHub"
style="display: inline-block; vertical-align: middle;">
<path fill-rule="evenodd" clip-rule="evenodd" d={Copy.GitHubIconPath} fill="currentColor"/>
</svg>
<span>View on GitHub</span>
</a>
</li>
<li style="margin-bottom: 0.5em;">
<a href="https://buymeacoffee.com/yourusername" style="text-decoration: none; display: inline-flex; align-items: center; gap: 0.5em;">
<span></span>
<span>Buy me a coffee</span>
</a>
</li>
</ul>
<hr style="margin: 2em 0; border-width: 1px">
<p style="font-style: italic; text-align: center;">Created with ❤️ for the Obsidian community</p>
{/if}
</div>
{/if}
</div>

View file

@ -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;
}
}

View file

@ -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();

View file

@ -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');
});
});
});