diff --git a/shell/src/app/chat/chat-cleaner/chat-cleaner.spec.ts b/shell/src/app/chat/chat-cleaner/chat-cleaner.spec.ts index 16a48a666..aabb71ac9 100644 --- a/shell/src/app/chat/chat-cleaner/chat-cleaner.spec.ts +++ b/shell/src/app/chat/chat-cleaner/chat-cleaner.spec.ts @@ -17,6 +17,7 @@ import {TestBed} from '@angular/core/testing'; import {describe, it, expect, beforeEach} from 'vitest'; import {ChatCleaner} from './chat-cleaner'; +import {MessageRole} from '../llm-client/llm-client'; describe('ChatCleaner', () => { let service: ChatCleaner; @@ -197,6 +198,73 @@ describe('ChatCleaner', () => { }); }); + describe('isUserLayoutSnapshot', () => { + it('returns true for unprompted USER messages with isSnapshot: true or JSON layout content', () => { + expect( + service.isUserLayoutSnapshot({ + role: MessageRole.USER, + content: '[{"version":"v0.9"}]', + isSnapshot: true, + }), + ).toBe(true); + expect( + service.isUserLayoutSnapshot({ + role: MessageRole.USER, + content: '[{"version":"v0.9"}]', + }), + ).toBe(true); + }); + + it('returns false for undefined, non-USER, prompted, attachment-bearing, or prose-prefixed messages', () => { + expect(service.isUserLayoutSnapshot(undefined)).toBe(false); + expect( + service.isUserLayoutSnapshot({ + role: MessageRole.MODEL, + content: '[{"version":"v0.9"}]', + }), + ).toBe(false); + expect( + service.isUserLayoutSnapshot({ + role: MessageRole.USER, + content: '[{"version":"v0.9"}]', + promptId: 'p1', + }), + ).toBe(false); + expect( + service.isUserLayoutSnapshot({ + role: MessageRole.USER, + content: '[{"version":"v0.9"}]', + attachments: [{name: 'img.png', mimeType: 'image/png', data: 'abc'}], + }), + ).toBe(false); + expect( + service.isUserLayoutSnapshot({ + role: MessageRole.USER, + content: '[{"version":"v0.9"}]', + isSnapshot: false, + }), + ).toBe(false); + expect( + service.isUserLayoutSnapshot({ + role: MessageRole.USER, + content: 'Fix this: [{"version":"v0.9"}]', + }), + ).toBe(false); + expect( + service.isUserLayoutSnapshot({ + role: MessageRole.USER, + content: undefined as unknown as string, + }), + ).toBe(false); + expect( + service.isUserLayoutSnapshot({ + role: MessageRole.USER, + content: null as unknown as string, + }), + ).toBe(false); + }); + }); + describe('Nullish and empty input resilience', () => { it('returns safe defaults when calling stripPulse on empty or nullish values', () => { expect(service.stripPulse('')).toBe(''); diff --git a/shell/src/app/chat/chat-cleaner/chat-cleaner.ts b/shell/src/app/chat/chat-cleaner/chat-cleaner.ts index f5b185b31..9e3186551 100644 --- a/shell/src/app/chat/chat-cleaner/chat-cleaner.ts +++ b/shell/src/app/chat/chat-cleaner/chat-cleaner.ts @@ -16,6 +16,7 @@ import {Injectable} from '@angular/core'; import {tryParseJsonArray} from '../../utils/json'; +import {LlmMessage, MessageRole} from '../llm-client/llm-client'; const MD_FENCE_REGEX = /```(?:jsonl?|jsonlines|a2ui|html|xml)?\s*([\s\S]*?)\s*```/gi; const TAG_REGEX = /<(thought|thinking|reasoning)>([\s\S]*?)(?:<\/\1>|$)/gi; @@ -164,4 +165,19 @@ export class ChatCleaner { const parsedArray = tryParseJsonArray(trimmed); return parsedArray.success && isLayoutArray(parsedArray.data); } + + /** + * Determines if a message is an unprompted USER canvas layout snapshot + * rather than a human prompt turn or a message with attachments. + */ + isUserLayoutSnapshot(msg?: LlmMessage): boolean { + if (!msg || msg.role !== MessageRole.USER || msg.promptId || msg.attachments?.length) { + return false; + } + if (msg.isSnapshot !== undefined) { + return msg.isSnapshot; + } + const trimmed = typeof msg.content === 'string' ? msg.content.trim() : ''; + return (trimmed.startsWith('[') || trimmed.startsWith('{')) && this.isLayoutSnapshot(trimmed); + } } diff --git a/shell/src/app/chat/chat-coordinator/chat-coordinator.spec.ts b/shell/src/app/chat/chat-coordinator/chat-coordinator.spec.ts index 759cfec8d..3a49aa343 100644 --- a/shell/src/app/chat/chat-coordinator/chat-coordinator.spec.ts +++ b/shell/src/app/chat/chat-coordinator/chat-coordinator.spec.ts @@ -255,7 +255,81 @@ describe('ChatCoordinator Pipeline & State Integration', () => { const prompt = service.systemPrompt(); expect(prompt).toContain('"Empty Catalog"'); - expect(prompt).toContain('"components": {}'); + expect(prompt).toContain('"components":{}'); + }); + + describe('Multimodal Guidelines Conditional Injection', () => { + beforeEach(() => { + catalogManagementMock.activeCatalog.set({ + catalogId: 'basic', + components: {Text: {}}, + }); + }); + + it('omits Multimodal & Image-to-UI Guidelines on text-only turns', async () => { + await service.submitPrompt('Build a login card'); + const messages = llmClientMock.chatStream.mock.calls[0][0]; + expect(messages[0].role).toBe(MessageRole.SYSTEM); + expect(messages[0].content).not.toContain('## Multimodal & Image-to-UI Guidelines'); + expect(service.systemPrompt()).not.toContain('## Multimodal & Image-to-UI Guidelines'); + }); + + it('omits Multimodal & Image-to-UI Guidelines when only non-image attachments (e.g., application/json) are present', async () => { + await service.submitPrompt('Import this data', [ + {name: 'data.json', mimeType: 'application/json', data: 'e30='}, + ]); + const messages = llmClientMock.chatStream.mock.calls[0][0]; + expect(messages[0].role).toBe(MessageRole.SYSTEM); + expect(messages[0].content).not.toContain('## Multimodal & Image-to-UI Guidelines'); + }); + + it('omits Multimodal & Image-to-UI Guidelines when image attachments exist only on MessageRole.ERROR messages', () => { + chatStateMock.setChatHistory([ + { + role: MessageRole.ERROR, + content: 'Failed turn', + attachments: [{name: 'screenshot.png', mimeType: 'image/png', data: 'base64'}], + }, + ]); + const context = service.getFullMessageContext(); + expect(context[0].content).not.toContain('## Multimodal & Image-to-UI Guidelines'); + expect(service.systemPrompt()).not.toContain('## Multimodal & Image-to-UI Guidelines'); + }); + + it('includes Multimodal & Image-to-UI Guidelines when an image attachment is present and updates systemPrompt() reactively', async () => { + await service.submitPrompt('Match this mockup', [ + {name: 'mockup.png', mimeType: 'image/png', data: 'base64'}, + ]); + const messages = llmClientMock.chatStream.mock.calls[0][0]; + expect(messages[0].role).toBe(MessageRole.SYSTEM); + expect(messages[0].content).toContain('## Multimodal & Image-to-UI Guidelines'); + expect(service.systemPrompt()).toContain('## Multimodal & Image-to-UI Guidelines'); + }); + + it('includes Multimodal & Image-to-UI Guidelines for screenshot.png, uppercase MIME types, and extension-inferred image files', async () => { + await service.submitPrompt('Fix spacing', [ + {name: 'screenshot.png', mimeType: '', data: 'base64'}, + ]); + expect(llmClientMock.chatStream.mock.calls[0][0][0].content).toContain( + '## Multimodal & Image-to-UI Guidelines', + ); + + service.startNewSession(); + await service.submitPrompt('Check uppercase MIME', [ + {name: 'upload', mimeType: 'IMAGE/PNG', data: 'base64'}, + ]); + expect(llmClientMock.chatStream.mock.calls[1][0][0].content).toContain( + '## Multimodal & Image-to-UI Guidelines', + ); + + service.startNewSession(); + await service.submitPrompt('Check extension fallback', [ + {name: 'wireframe.webp', mimeType: 'application/octet-stream', data: 'base64'}, + ]); + expect(llmClientMock.chatStream.mock.calls[2][0][0].content).toContain( + '## Multimodal & Image-to-UI Guidelines', + ); + }); }); describe('editing an imported demo', () => { @@ -377,10 +451,13 @@ describe('ChatCoordinator Pipeline & State Integration', () => { expect(service.pipelineStatus()).toBe(PipelineStatus.READY); expect(service.isProgrammaticStreamActive()).toBe(false); - // Verify raw content streamed directly inside model's turn logs + // Verify compact JSON content stored in model's turn log const updatedHistory = chatStateMock.chatHistory(); expect(updatedHistory[1].role).toBe(MessageRole.MODEL); - expect(updatedHistory[1].content).toContain('"createSurface": {'); + expect(updatedHistory[1].content).toBe( + '[{"version":"v0.9","createSurface":{"surfaceId":"s1","catalogId":"basic"}}]', + ); + expect(updatedHistory[1].content).not.toContain('\n'); expect(updatedHistory[1].content).not.toContain('●●●'); // Verify layout committed back in a single commit transaction @@ -701,10 +778,12 @@ describe('ChatCoordinator Pipeline & State Integration', () => { expect(stream.cancel).toHaveBeenCalledTimes(1); expect(trackChatCancel).not.toHaveBeenCalled(); expect(chatStateMock.chatHistory()).toEqual([ - {role: MessageRole.USER, content: 'Make a card', promptId: 'prompt-1'}, + {role: MessageRole.USER, content: 'Make a card', isSnapshot: false, promptId: 'prompt-1'}, { role: MessageRole.MODEL, content: '*This response was stopped because the renderer changed.*', + isCancelled: true, + promptId: 'prompt-1', }, ]); expect(service.pipelineStatus()).toBe(PipelineStatus.IDLE); @@ -895,9 +974,9 @@ I should generate a text field. const history = chatStateMock.chatHistory(); const lastMessage = history[history.length - 1]; - // Extract the updated content - it should be cleanly parsed JSON text structure + // Extract the updated content - it should be cleanly parsed compact JSON text structure // that is successfully parsed from the payload. - expect(lastMessage.content).toContain('"version": "v0.9"'); + expect(lastMessage.content).toContain('"version":"v0.9"'); expect(lastMessage.content).toContain('"createSurface"'); expect(lastMessage.content).toContain('"TextField"'); @@ -1316,4 +1395,389 @@ I'll output bad JSON. expect(secondCancel).toHaveBeenCalledTimes(1); }); }); + + describe('Failed Turn Context Exclusion', () => { + it('excludes failed user prompt turns and their attachments from LLM context on retry while preserving them in chatHistory', async () => { + const screenshotAttachment = { + name: 'screenshot.png', + mimeType: 'image/png', + data: 'base64-image-data', + }; + llmClientMock.chatStream.mockRejectedValueOnce(new Error('HTTP 503 Service Unavailable')); + + await service.submitPrompt('Build this complex dashboard', [screenshotAttachment]); + + const historyAfterFailure = chatStateMock.chatHistory(); + expect(historyAfterFailure).toHaveLength(2); + expect(historyAfterFailure[0].role).toBe(MessageRole.USER); + expect(historyAfterFailure[1].role).toBe(MessageRole.ERROR); + const failedPromptId = historyAfterFailure[0].promptId; + expect(failedPromptId).toBeDefined(); + + // Restore working stream for retry + const validResponse = + '{"version": "v0.9", "createSurface": {"surfaceId": "s1", "catalogId": "basic"}}\n'; + llmClientMock.chatStream.mockResolvedValueOnce({ + contentStream: createMockStream([validResponse]), + complete: Promise.resolve(validResponse), + }); + + await service.submitPrompt('Build this complex dashboard', [screenshotAttachment], { + retryOfPromptId: failedPromptId, + }); + + const retryMessages = llmClientMock.chatStream.mock.calls[1][0]; + const userMessagesSentToLlm = retryMessages.filter(m => m.role === MessageRole.USER); + expect(userMessagesSentToLlm).toHaveLength(1); + expect(userMessagesSentToLlm[0].content).toBe('Build this complex dashboard'); + expect(userMessagesSentToLlm[0].promptId).not.toBe(failedPromptId); + + // UI chatHistory still retains the failed user turn and error message + expect(chatStateMock.chatHistory().some(m => m.role === MessageRole.ERROR)).toBe(true); + expect( + chatStateMock + .chatHistory() + .filter(m => m.role === MessageRole.USER && m.content === 'Build this complex dashboard'), + ).toHaveLength(2); + }); + + it('preserves a succeeded retry that reuses promptId in getFullMessageContext() on subsequent follow-up turns', async () => { + llmClientMock.chatStream.mockRejectedValueOnce(new Error('HTTP 503 Service Unavailable')); + await service.submitPrompt('Build a profile card', [], {promptId: 'reused-prompt-id'}); + + const validResponse = + '{"version": "v0.9", "createSurface": {"surfaceId": "s1", "catalogId": "basic"}}\n'; + llmClientMock.chatStream.mockResolvedValue({ + contentStream: createMockStream([validResponse]), + complete: Promise.resolve(validResponse), + }); + + // Retry reusing the same promptId and succeed + await service.submitPrompt('Build a profile card', [], { + promptId: 'reused-prompt-id', + retryOfPromptId: 'reused-prompt-id', + }); + + // Submit a subsequent follow-up turn + await service.submitPrompt('Now add an avatar', [], {promptId: 'followup-prompt-id'}); + + const followupMessages = llmClientMock.chatStream.mock.calls[2][0]; + const userMessages = followupMessages.filter(m => m.role === MessageRole.USER); + expect(userMessages.map(m => m.content)).toEqual([ + 'Build a profile card', + 'Now add an avatar', + ]); + }); + + it('excludes cancelled turns (*You stopped this response.*) and their preceding user messages/attachments from subsequent LLM requests', async () => { + catalogManagementMock.activeCatalog.set({ + catalogId: 'basic', + components: {Text: {}}, + }); + chatStateMock.setChatHistory([ + { + role: MessageRole.USER, + content: '[{"version":"v0.9","createSurface":{"surfaceId":"s1","catalogId":"basic"}}]', + }, + { + role: MessageRole.USER, + content: 'Cancelled image prompt', + promptId: 'cancelled-id', + attachments: [{name: 'screenshot.png', mimeType: 'image/png', data: 'base64'}], + }, + {role: MessageRole.MODEL, content: '*You stopped this response.*'}, + ]); + + await service.submitPrompt('Text-only follow-up after cancel'); + + const sentMessages = llmClientMock.chatStream.mock.calls[0][0]; + expect(sentMessages[0].role).toBe(MessageRole.SYSTEM); + // Since the cancelled turn with the image attachment was excluded, multimodal guidelines should be omitted + expect(sentMessages[0].content).not.toContain('## Multimodal & Image-to-UI Guidelines'); + expect(sentMessages.some(m => m.content === '*You stopped this response.*')).toBe(false); + expect(sentMessages.some(m => m.content === 'Cancelled image prompt')).toBe(false); + expect(sentMessages.filter(m => m.role === MessageRole.USER).map(m => m.content)).toEqual([ + '[{"version":"v0.9","createSurface":{"surfaceId":"s1","catalogId":"basic"}}]', + 'Text-only follow-up after cancel', + ]); + }); + + it('excludes un-tagged failed user messages immediately preceding an error while preserving layout snapshots', () => { + chatStateMock.setChatHistory([ + { + role: MessageRole.USER, + content: '[{"version":"v0.9","createSurface":{"surfaceId":"s1","catalogId":"basic"}}]', + isSnapshot: true, + }, + {role: MessageRole.USER, content: 'Failed prompt without promptId'}, + {role: MessageRole.ERROR, content: 'Service unavailable'}, + {role: MessageRole.USER, content: 'Successful followup', promptId: 'valid-id'}, + ]); + + const context = service.getFullMessageContext(); + const userContents = context.filter(m => m.role === MessageRole.USER).map(m => m.content); + expect(userContents).toEqual([ + '[{"version":"v0.9","createSurface":{"surfaceId":"s1","catalogId":"basic"}}]', + 'Successful followup', + ]); + }); + + it('preserves an earlier succeeded turn when a later retry reusing the same promptId fails', async () => { + const validResponse = + '{"version": "v0.9", "createSurface": {"surfaceId": "s1", "catalogId": "basic"}}\n'; + llmClientMock.chatStream.mockResolvedValueOnce({ + contentStream: createMockStream([validResponse]), + complete: Promise.resolve(validResponse), + }); + + // Turn 1 succeeds with promptId 'p1' + await service.submitPrompt('Initial succeeded prompt', [], {promptId: 'p1'}); + + // Turn 2 reuses promptId 'p1' and fails + llmClientMock.chatStream.mockRejectedValueOnce(new Error('HTTP 503 Service Unavailable')); + await service.submitPrompt('Retried prompt that fails', [], { + promptId: 'p1', + retryOfPromptId: 'p1', + }); + + const context = service.getFullMessageContext(); + const nonSystem = context.filter(m => m.role !== MessageRole.SYSTEM); + expect(nonSystem.map(m => ({role: m.role, content: m.content}))).toEqual([ + {role: MessageRole.USER, content: 'Initial succeeded prompt'}, + { + role: MessageRole.MODEL, + content: '[{"version":"v0.9","createSurface":{"surfaceId":"s1","catalogId":"basic"}}]', + }, + ]); + }); + + it('excludes MODEL turns with parseError and their preceding USER prompt and image attachments from context and systemPrompt()', async () => { + catalogManagementMock.activeCatalog.set({ + catalogId: 'basic', + components: {Text: {}}, + }); + const unparseableRawText = 'bad\n{"version": "v'; + llmClientMock.chatStream.mockResolvedValueOnce({ + contentStream: createMockStream([unparseableRawText]), + complete: Promise.resolve(unparseableRawText), + }); + + await service.submitPrompt('Broken image prompt', [ + {name: 'screenshot.png', mimeType: 'image/png', data: 'base64'}, + ]); + + const context = service.getFullMessageContext(); + expect(context).toHaveLength(1); + expect(context[0].role).toBe(MessageRole.SYSTEM); + expect(context[0].content).not.toContain('## Multimodal & Image-to-UI Guidelines'); + expect(service.systemPrompt()).not.toContain('## Multimodal & Image-to-UI Guidelines'); + }); + + it('excludes a cancelled user prompt even when the prompt text contains an A2UI JSON snippet', () => { + chatStateMock.setChatHistory([ + { + role: MessageRole.USER, + content: + 'Please fix this layout:\n```json\n[{"version":"v0.9","createSurface":{"surfaceId":"s1","catalogId":"basic"}}]\n```', + promptId: 'cancelled-json-prompt', + }, + { + role: MessageRole.MODEL, + content: '*You stopped this response.*', + isCancelled: true, + promptId: 'cancelled-json-prompt', + }, + { + role: MessageRole.USER, + content: 'Fresh prompt', + promptId: 'fresh-prompt', + }, + ]); + + const context = service.getFullMessageContext(); + const userMessages = context.filter(m => m.role === MessageRole.USER); + expect(userMessages.map(m => m.content)).toEqual(['Fresh prompt']); + }); + + it('deduplicates consecutive layout snapshots left adjacent after stripping an intervening failed turn', () => { + chatStateMock.setChatHistory([ + { + role: MessageRole.USER, + content: '[{"version":"v0.9","createSurface":{"surfaceId":"s1","catalogId":"basic"}}]', + isSnapshot: true, + }, + { + role: MessageRole.USER, + content: 'Failed prompt between snapshots', + promptId: 'failed-p1', + }, + { + role: MessageRole.ERROR, + content: '503 Service Unavailable', + promptId: 'failed-p1', + }, + { + role: MessageRole.USER, + content: '[{"version":"v0.9","createSurface":{"surfaceId":"s2","catalogId":"basic"}}]', + isSnapshot: true, + }, + { + role: MessageRole.USER, + content: 'Follow-up prompt', + promptId: 'valid-p2', + }, + ]); + + const context = service.getFullMessageContext(); + const userMessages = context.filter(m => m.role === MessageRole.USER); + expect(userMessages.map(m => m.content)).toEqual([ + '[{"version":"v0.9","createSurface":{"surfaceId":"s2","catalogId":"basic"}}]', + 'Follow-up prompt', + ]); + }); + + it('updates the matching MODEL placeholder by promptId when a USER layout snapshot is appended mid-stream', async () => { + let resolveStream!: (val: string) => void; + const streamPromise = new Promise(resolve => { + resolveStream = resolve; + }); + async function* delayedGenerator() { + const chunk = await streamPromise; + yield chunk; + } + llmClientMock.chatStream.mockResolvedValueOnce({ + contentStream: delayedGenerator(), + complete: streamPromise, + }); + + const submitPromise = service.submitPrompt('Prompt with mid-stream snapshot', [], { + promptId: 'turn-mid-snap', + }); + + const historyDuringStream = chatStateMock.chatHistory(); + expect(historyDuringStream[0]).toEqual( + expect.objectContaining({ + role: MessageRole.USER, + content: 'Prompt with mid-stream snapshot', + isSnapshot: false, + promptId: 'turn-mid-snap', + }), + ); + + // Simulate a USER layout snapshot appended after the MODEL placeholder while stream is active + chatStateMock.updateChatHistory(h => [ + ...h, + { + role: MessageRole.USER, + content: + '[{"version":"v0.9","createSurface":{"surfaceId":"interleaved","catalogId":"basic"}}]', + isSnapshot: true, + }, + ]); + + const validResponse = + '{"version":"v0.9","createSurface":{"surfaceId":"s1","catalogId":"basic"}}'; + resolveStream(validResponse); + await submitPromise; + + const historyAfterComplete = chatStateMock.chatHistory(); + const modelTurn = historyAfterComplete.find( + m => m.role === MessageRole.MODEL && m.promptId === 'turn-mid-snap', + ); + expect(modelTurn?.content).toBe(`[${validResponse}]`); + }); + + it('excludes a failed or cancelled USER prompt while preserving an interleaved USER layout snapshot', () => { + chatStateMock.setChatHistory([ + { + role: MessageRole.USER, + content: 'Failed prompt before snapshot', + isSnapshot: false, + promptId: 'p-failed', + }, + { + role: MessageRole.USER, + content: + '[{"version":"v0.9","createSurface":{"surfaceId":"kept-snap","catalogId":"basic"}}]', + isSnapshot: true, + }, + { + role: MessageRole.ERROR, + content: '503 Service Unavailable', + promptId: 'p-failed', + }, + { + role: MessageRole.USER, + content: 'Cancelled prompt before snapshot', + isSnapshot: false, + promptId: 'p-cancelled', + }, + { + role: MessageRole.USER, + content: + '[{"version":"v0.9","createSurface":{"surfaceId":"kept-snap-2","catalogId":"basic"}}]', + isSnapshot: true, + }, + { + role: MessageRole.MODEL, + content: '*You stopped this response.*', + isCancelled: true, + promptId: 'p-cancelled', + }, + { + role: MessageRole.USER, + content: 'Active prompt', + isSnapshot: false, + promptId: 'p-active', + }, + ]); + + const context = service.getFullMessageContext(); + const userMessages = context.filter(m => m.role === MessageRole.USER); + expect(userMessages.map(m => m.content)).toEqual([ + '[{"version":"v0.9","createSurface":{"surfaceId":"kept-snap-2","catalogId":"basic"}}]', + 'Active prompt', + ]); + }); + + it('appends a new ERROR bubble instead of overwriting an earlier completed MODEL turn with a different promptId when no matching MODEL placeholder exists', async () => { + chatStateMock.setChatHistory([ + { + role: MessageRole.USER, + content: 'Earlier succeeded prompt', + isSnapshot: false, + promptId: 'earlier-turn', + }, + { + role: MessageRole.MODEL, + content: '[{"version":"v0.9","createSurface":{"surfaceId":"s1","catalogId":"basic"}}]', + promptId: 'earlier-turn', + }, + ]); + + llmClientMock.chatStream.mockImplementationOnce(async () => { + // Remove the newly appended 'later-turn' MODEL placeholder (and USER prompt) so the last + // entry in chatHistory is the earlier completed MODEL turn with promptId 'earlier-turn' + chatStateMock.updateChatHistory(h => h.filter(m => m.promptId !== 'later-turn')); + throw new Error('HTTP 503 Service Unavailable'); + }); + + await service.submitPrompt('Later failed prompt', [], {promptId: 'later-turn'}); + + const history = chatStateMock.chatHistory(); + expect(history).toHaveLength(3); + expect(history[1]).toEqual({ + role: MessageRole.MODEL, + content: '[{"version":"v0.9","createSurface":{"surfaceId":"s1","catalogId":"basic"}}]', + promptId: 'earlier-turn', + }); + expect(history[2]).toEqual( + expect.objectContaining({ + role: MessageRole.ERROR, + promptId: 'later-turn', + originalPrompt: 'Later failed prompt', + }), + ); + }); + }); }); diff --git a/shell/src/app/chat/chat-coordinator/chat-coordinator.ts b/shell/src/app/chat/chat-coordinator/chat-coordinator.ts index c9f4e2cdf..718446b22 100644 --- a/shell/src/app/chat/chat-coordinator/chat-coordinator.ts +++ b/shell/src/app/chat/chat-coordinator/chat-coordinator.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import {inject, Injectable, signal} from '@angular/core'; +import {computed, inject, Injectable, signal} from '@angular/core'; import {takeUntilDestroyed, toObservable} from '@angular/core/rxjs-interop'; import {skip} from 'rxjs/operators'; import {PreviewBridgeMessageType} from 'a2ui-bridge'; @@ -23,6 +23,7 @@ import {CrossFrameValidator} from '../../shell/cross-frame-validator/cross-frame import {CatalogManagement} from '../../storage/catalog-management/catalog-management'; import {PromptTurnType, UsageTrackingService} from '../../usage-tracking/usage-tracking.service'; import {formatJson, tryParseJsonArray} from '../../utils/json'; +import {isImageAttachment as isA2aImageAttachment} from '../a2a/a2a-attachments'; import {ChatState, LlmLogType} from '../chat-state/chat-state'; import { Attachment, @@ -31,6 +32,7 @@ import { LlmMessage, LlmStreamResponse, MessageRole, + STOPPED_RESPONSE_MESSAGE, } from '../llm-client/llm-client'; import {PipelineStatus} from '../pipeline-status/pipeline-status'; import {StateSync} from '../state-sync/state-sync'; @@ -55,9 +57,6 @@ enum CancelReason { RENDERER_CHANGED = 'rendererChanged', } -/** Shown in place of a response the user cancelled. */ -const USER_CANCELLED_NOTE = '*You stopped this response.*'; - /** Shown in place of a response cancelled because the renderer changed. */ const RENDERER_CHANGED_NOTE = '*This response was stopped because the renderer changed.*'; @@ -95,11 +94,22 @@ export class ChatCoordinator { /** Turn index counter for telemetry. */ readonly currentTurnIndex = signal(0); + private readonly conversationalMessages = computed(() => + this.getConversationalMessages(this.chatState.chatHistory()), + ); + /** * A dynamic, reactive, computed signal property constructing conformed JSON * catalog schema specifications system instructions. */ - readonly systemPrompt = this.promptFactory.systemPrompt; + readonly systemPrompt = computed(() => { + const hasImageAttachment = this.conversationalMessages().some(m => + m.attachments?.some(a => this.isImageAttachment(a)), + ); + return this.promptFactory.buildSystemPrompt({ + includeMultimodalGuidelines: hasImageAttachment, + }); + }); private activePromptId: string | null = null; @@ -167,6 +177,101 @@ export class ChatCoordinator { this.chatState.setProgrammaticStreamActive(false); } + private isImageAttachment(attachment: Attachment): boolean { + return isA2aImageAttachment(attachment.mimeType, attachment.name); + } + + private isFailedOrCancelledOutcome(msg?: LlmMessage): boolean { + if (!msg) { + return false; + } + if (msg.role === MessageRole.ERROR) { + return true; + } + if (msg.role === MessageRole.MODEL) { + return ( + Boolean(msg.isCancelled) || + msg.content === STOPPED_RESPONSE_MESSAGE || + msg.content === RENDERER_CHANGED_NOTE || + Boolean(msg.parseError) + ); + } + return false; + } + + /** + * Pairs each USER prompt turn positionally with its immediately following + * non-snapshot outcome message so that retries reusing an earlier turn's + * `promptId` only exclude the specific attempt that failed or was cancelled, + * without orphaning or dropping an earlier succeeded turn with the same ID. + */ + private getConversationalMessages(history: LlmMessage[]): LlmMessage[] { + const filtered = history.filter((m, idx) => { + if (this.isFailedOrCancelledOutcome(m)) { + return false; + } + if (m.role === MessageRole.USER && !this.chatCleaner.isUserLayoutSnapshot(m)) { + let outcomeIdx = idx + 1; + while ( + outcomeIdx < history.length && + this.chatCleaner.isUserLayoutSnapshot(history[outcomeIdx]) + ) { + outcomeIdx++; + } + const outcomeMsg = history[outcomeIdx]; + if ( + this.isFailedOrCancelledOutcome(outcomeMsg) && + (!outcomeMsg?.promptId || !m.promptId || outcomeMsg.promptId === m.promptId) + ) { + return false; + } + } + return true; + }); + + // Deduplicate adjacent layout snapshots left behind when an intervening failed/cancelled turn is stripped + return filtered.filter( + (m, idx, arr) => + !this.chatCleaner.isUserLayoutSnapshot(m) || + !this.chatCleaner.isUserLayoutSnapshot(arr[idx + 1]), + ); + } + + /** + * Searches backward from the end of `history` so the most recent MODEL + * placeholder matching `promptId` is targeted even if a retry reuses an + * earlier turn's `promptId` or a layout snapshot is appended mid-stream. + */ + private findTargetModelMessageIndex(history: LlmMessage[], promptId?: string): number { + if (promptId) { + for (let i = history.length - 1; i >= 0; i--) { + if (history[i].role === MessageRole.MODEL && history[i].promptId === promptId) { + return i; + } + } + } + const lastIdx = history.length - 1; + return lastIdx >= 0 && + history[lastIdx].role === MessageRole.MODEL && + (!promptId || !history[lastIdx].promptId) + ? lastIdx + : -1; + } + + private updateTargetModelMessage(promptId: string | undefined, patch: Partial): void { + this.chatState.updateChatHistory(history => { + const updated = [...history]; + const targetIdx = this.findTargetModelMessageIndex(updated, promptId); + if (targetIdx >= 0) { + updated[targetIdx] = { + ...updated[targetIdx], + ...patch, + }; + } + return updated; + }); + } + /** * Constructs System instructions prepended message logs context. */ @@ -174,9 +279,9 @@ export class ChatCoordinator { return [ { role: MessageRole.SYSTEM, - content: this.promptFactory.systemPrompt(), + content: this.systemPrompt(), }, - ...this.chatState.chatHistory().filter(m => m.role !== MessageRole.ERROR), + ...this.conversationalMessages(), ]; } @@ -225,12 +330,8 @@ export class ChatCoordinator { const catalogObj = this.catalogManagement.activeCatalog(); const catalogId = catalogObj ? catalogObj.catalogId || catalogObj.$id || '' : ''; - const hasScreenshot = attachments.some( - a => a.name === 'screenshot.png' || a.mimeType?.startsWith('image/'), - ); - const nonScreenshotAttachments = attachments.filter( - a => a.name !== 'screenshot.png' && !a.mimeType?.startsWith('image/'), - ); + const hasScreenshot = attachments.some(a => this.isImageAttachment(a)); + const nonScreenshotAttachments = attachments.filter(a => !this.isImageAttachment(a)); if (isRetry) { return this.usageTrackingService.trackChatRetry({ @@ -286,6 +387,7 @@ export class ChatCoordinator { role: MessageRole.USER, content: trimmed, attachments: attachments.length > 0 ? attachments : undefined, + isSnapshot: false, promptId, }, ]); @@ -301,6 +403,7 @@ export class ChatCoordinator { { role: MessageRole.MODEL, content: this.chatCleaner.appendPulse(''), + promptId, }, ]); @@ -336,17 +439,11 @@ export class ChatCoordinator { } // Update history bubble in real-time with trailing pulse indicator - this.chatState.updateChatHistory(history => { - const updated = [...history]; - const lastIdx = updated.length - 1; - if (updated[lastIdx]?.role === MessageRole.MODEL) { - updated[lastIdx] = { - role: MessageRole.MODEL, - content: this.chatCleaner.appendPulse(accumulatedRawText), - thinking: accumulatedThinking, - }; - } - return updated; + this.updateTargetModelMessage(promptId, { + role: MessageRole.MODEL, + content: this.chatCleaner.appendPulse(accumulatedRawText), + thinking: accumulatedThinking, + promptId, }); } @@ -356,17 +453,11 @@ export class ChatCoordinator { // Log the raw LLM response telemetry this.chatState.addRawLlmLog(LlmLogType.RESPONSE, finalRawText); - this.chatState.updateChatHistory(history => { - const updated = [...history]; - const lastIdx = updated.length - 1; - if (updated[lastIdx]?.role === MessageRole.MODEL) { - updated[lastIdx] = { - role: MessageRole.MODEL, - content: finalRawText, - thinking: accumulatedThinking, - }; - } - return updated; + this.updateTargetModelMessage(promptId, { + role: MessageRole.MODEL, + content: finalRawText, + thinking: accumulatedThinking, + promptId, }); this.chatState.setPipelineStatus(PipelineStatus.RECEIVED_RAW); @@ -378,19 +469,13 @@ export class ChatCoordinator { if (err && typeof err === 'object' && 'name' in err && err.name === CANCEL_ERROR_NAME) { this.finalizeStream(PipelineStatus.IDLE); // Replace trailing pulse or partial JSON with stopped message, and force non-snapshot - this.chatState.updateChatHistory(history => { - const updated = [...history]; - const lastIdx = updated.length - 1; - if (updated[lastIdx]?.role === MessageRole.MODEL) { - updated[lastIdx] = { - ...updated[lastIdx], - content: - this.cancelReason === CancelReason.RENDERER_CHANGED - ? RENDERER_CHANGED_NOTE - : USER_CANCELLED_NOTE, - }; - } - return updated; + this.updateTargetModelMessage(promptId, { + content: + this.cancelReason === CancelReason.RENDERER_CHANGED + ? RENDERER_CHANGED_NOTE + : STOPPED_RESPONSE_MESSAGE, + isCancelled: true, + promptId, }); } else { this.handleConnectivityError(err, trimmed, attachments, promptId); @@ -432,14 +517,7 @@ export class ChatCoordinator { snippet: parseResult.snippet, }); - this.chatState.updateChatHistory(history => { - const updated = [...history]; - const lastIdx = updated.length - 1; - if (updated[lastIdx]?.role === MessageRole.MODEL) { - updated[lastIdx] = {...updated[lastIdx], parseError: parseResult}; - } - return updated; - }); + this.updateTargetModelMessage(promptId, {parseError: parseResult}); this.finalizeStream(PipelineStatus.IDLE); return; } @@ -487,23 +565,16 @@ export class ChatCoordinator { parsedBlocks = this.resolveLayoutUpdate(parsedBlocks); this.chatState.setPipelineStatus(PipelineStatus.READY); - // Turn list of updates back into raw formatted JSON text to write to - // editor draft + // Serialize compact JSON for LLM chat history context while keeping + // formatted JSON for the Monaco editor draft. + const compactLayoutText = JSON.stringify(parsedBlocks); const finalLayoutText = formatJson(parsedBlocks); - // Update latest MODEL turn message in history to normalized JSON string - this.chatState.updateChatHistory(history => { - const updated = [...history]; - const lastIdx = updated.length - 1; - if (updated[lastIdx]?.role === MessageRole.MODEL) { - updated[lastIdx] = { - ...updated[lastIdx], - content: finalLayoutText, - isSnapshot: true, - componentCount, - }; - } - return updated; + // Update latest MODEL turn message in history to compact JSON string + this.updateTargetModelMessage(promptId, { + content: compactLayoutText, + isSnapshot: true, + componentCount, }); // Commit layout synchronously to editor viewport before releasing @@ -636,7 +707,7 @@ export class ChatCoordinator { this.chatState.updateChatHistory(history => { const updated = [...history]; - const lastIdx = updated.length - 1; + const targetIdx = this.findTargetModelMessageIndex(updated, promptId); const errorBubble = { role: MessageRole.ERROR, content: redactedErrorMessage, @@ -647,8 +718,8 @@ export class ChatCoordinator { promptId, ...(parsed.isRetryable ? {isRetryable: true, originalPrompt, attachments} : {}), }; - if (lastIdx >= 0 && updated[lastIdx].role === MessageRole.MODEL) { - updated[lastIdx] = errorBubble; + if (targetIdx >= 0) { + updated[targetIdx] = errorBubble; return updated; } updated.push(errorBubble); diff --git a/shell/src/app/chat/chat-panel/chat-panel-base.ts b/shell/src/app/chat/chat-panel/chat-panel-base.ts index 313a3cc6b..765c35c71 100644 --- a/shell/src/app/chat/chat-panel/chat-panel-base.ts +++ b/shell/src/app/chat/chat-panel/chat-panel-base.ts @@ -25,6 +25,7 @@ import { linkedSignal, } from '@angular/core'; import {takeUntilDestroyed} from '@angular/core/rxjs-interop'; +import {isImageAttachment} from '../a2a/a2a-attachments'; import {FileIngestionService, AttachedFile} from '../file-ingestion/file-ingestion.service'; import {MatDialog} from '@angular/material/dialog'; import {AppConfigProvider, AuthType} from '../../settings/app-config-provider/app-config-provider'; @@ -116,7 +117,15 @@ export abstract class ChatPanelBase { * Reactively computed dynamic system prompt instructions spec viewport * text. */ - protected readonly systemPrompt = this.chatCoordinator.systemPrompt; + protected readonly systemPrompt = computed(() => { + const hasStagedImage = + this.includeScreenshot() || + this.attachedFiles().some(f => isImageAttachment(f.mimeType, f.name)); + if (hasStagedImage) { + return this.promptFactory.buildSystemPrompt({includeMultimodalGuidelines: true}); + } + return this.chatCoordinator.systemPrompt(); + }); protected readonly hasCustomInstructions = this.promptFactory.hasCustomInstructions; protected readonly activeCustomPreset = this.promptFactory.activePreset; protected readonly isHandshakeComplete = computed( @@ -185,20 +194,23 @@ export abstract class ChatPanelBase { componentCount: null, }; } - if (m.isSnapshot !== undefined) { + if ( + m.role === MessageRole.ERROR || + m.isSnapshot === false || + (m.role === MessageRole.USER && !this.chatCleaner.isUserLayoutSnapshot(m)) || + (m.isSnapshot === true && m.componentCount != null) + ) { return { ...m, - isSnapshot: m.isSnapshot, + isSnapshot: Boolean(m.isSnapshot), isStreaming: false, componentCount: m.componentCount ?? null, }; } const cleaned = m.content ? this.chatCleaner.cleanPayload(m.content) : ''; const parseResult = cleaned ? parseAndHealJsonLines(cleaned) : null; - let isSnapshot = false; - if (m.content && parseResult && parseResult.success) { - isSnapshot = !parseResult.isConversational; - } + const isSnapshot = + Boolean(m.isSnapshot) || Boolean(parseResult?.success && !parseResult.isConversational); if (isSnapshot && m.content) { return { ...m, @@ -327,9 +339,8 @@ export abstract class ChatPanelBase { */ protected getBubbleClass(message: LlmMessage): string { if (message.role === MessageRole.USER) { - return message.content && this.chatCleaner.isLayoutSnapshot(message.content) - ? 'bubble-user bubble-layout' - : 'bubble-user bubble-text'; + const isLayoutSnapshot = message.isSnapshot ?? this.chatCleaner.isUserLayoutSnapshot(message); + return isLayoutSnapshot ? 'bubble-user bubble-layout' : 'bubble-user bubble-text'; } if (message.role === MessageRole.MODEL) { return 'bubble-model'; @@ -356,7 +367,7 @@ export abstract class ChatPanelBase { */ protected showSystemInstructions(): void { this.dialog.open(SystemInstructionsDialog, { - data: this.chatCoordinator.systemPrompt(), + data: this.systemPrompt(), maxWidth: '90vw', }); } diff --git a/shell/src/app/chat/chat-panel/chat-panel.spec.ts b/shell/src/app/chat/chat-panel/chat-panel.spec.ts index 0a54c84fa..52a2e166d 100644 --- a/shell/src/app/chat/chat-panel/chat-panel.spec.ts +++ b/shell/src/app/chat/chat-panel/chat-panel.spec.ts @@ -110,6 +110,9 @@ class MockChatCoordinator { class MockChatPromptFactoryService { readonly systemPrompt = signal('Initial system prompt instructions block'); + buildSystemPrompt = vi.fn((opts?: {includeMultimodalGuidelines?: boolean}) => + opts?.includeMultimodalGuidelines ? 'multimodal prompt' : this.systemPrompt(), + ); readonly customInstructionsState = signal({ presets: [], activePresetId: null, @@ -221,8 +224,9 @@ describe('ChatPanel Gemini Dialogue Panel Integration', () => { { role: MessageRole.USER, content: - '{"version": "v0.9", "updateComponents": {"surfaceId": "s1", ' + - '"components": [{"id":"b1","component":"Button"}]}}', + '[{"version":"v0.9","updateComponents":{"surfaceId":"s1",' + + '"components":[{"id":"b1","component":"Button"}]}}]', + isSnapshot: true, }, { role: MessageRole.MODEL, @@ -244,7 +248,7 @@ describe('ChatPanel Gemini Dialogue Panel Integration', () => { expect(bubbles[0]).toBe('please append a basic search form widget'); expect(bubbleTypes[0]).toBe('human-text'); - // Bubble 2: Layout snapshot block + // Bubble 2: Layout snapshot block stamped with isSnapshot: true and no pre-populated componentCount expect(bubbleHeaders[1]).toBe('Canvas Revision Snapshot'); expect(bubbles[1]).toBe('Received 1 A2UI JSON Components'); expect(bubbleTypes[1]).toBe('layout-snapshot'); @@ -256,10 +260,10 @@ describe('ChatPanel Gemini Dialogue Panel Integration', () => { }, ); - it('classifies turns wrapped in ```jsonl code blocks as layout snapshots and calculates component counts', async () => { + it('classifies model turns wrapped in ```jsonl code blocks as layout snapshots and calculates component counts', async () => { const historyMocks: LlmMessage[] = [ { - role: MessageRole.USER, + role: MessageRole.MODEL, content: '```jsonl\n{"version": "v0.9", "updateComponents": {"surfaceId": "s1", "components": [{"id":"b1","component":"Button"}, {"id":"b2","component":"Button"}]}}\n```', }, @@ -272,14 +276,14 @@ describe('ChatPanel Gemini Dialogue Panel Integration', () => { const bubbleTypes = await harness.getBubbleTypes(); expect(bubbles.length).toBe(1); - expect(bubbleTypes[0]).toBe('layout-snapshot'); + expect(bubbleTypes[0]).toBe('model-response'); expect(bubbles[0]).toBe('Received 2 A2UI JSON Components'); }); - it('classifies turns containing preamble text and ```jsonl code blocks as layout snapshots', async () => { + it('classifies model turns containing preamble text and ```jsonl code blocks as layout snapshots', async () => { const historyMocks: LlmMessage[] = [ { - role: MessageRole.USER, + role: MessageRole.MODEL, content: 'Here is your layout:\n```jsonl\n{"version": "v0.9", "updateComponents": {"surfaceId": "s1", "components": [{"id":"b1","component":"Button"}]}}\n```', }, @@ -292,10 +296,43 @@ describe('ChatPanel Gemini Dialogue Panel Integration', () => { const bubbleTypes = await harness.getBubbleTypes(); expect(bubbles.length).toBe(1); - expect(bubbleTypes[0]).toBe('layout-snapshot'); + expect(bubbleTypes[0]).toBe('model-response'); expect(bubbles[0]).toBe('Received 1 A2UI JSON Components'); }); + it('renders user prompts containing fenced A2UI JSON snippets as conversational user messages rather than layout snapshots', async () => { + const promptWithFencedA2ui = + 'Please fix this button:\n```jsonl\n{"version": "v0.9", "updateComponents": {"surfaceId": "s1", "components": [{"id":"b1","component":"Button"}]}}\n```'; + const fencedOnlyPrompt = + '```jsonl\n{"version": "v0.9", "updateComponents": {"surfaceId": "s1", "components": [{"id":"b1","component":"Button"}]}}\n```'; + const historyMocks: LlmMessage[] = [ + { + role: MessageRole.USER, + content: promptWithFencedA2ui, + isSnapshot: false, + promptId: 'prompt-1', + }, + { + // Legacy user prompt without explicit isSnapshot or promptId + role: MessageRole.USER, + content: fencedOnlyPrompt, + }, + ]; + + chatStateMock.chatHistory.set(historyMocks); + fixture.detectChanges(); + + const bubbles = await harness.getBubblesText(); + const bubbleTypes = await harness.getBubbleTypes(); + const bubbleHeaders = await harness.getBubbleHeaders(); + + expect(bubbles.length).toBe(2); + expect(bubbleHeaders).toEqual(['You', 'You']); + expect(bubbleTypes).toEqual(['human-text', 'human-text']); + expect(bubbles[0]).toContain('Please fix this button:'); + expect(bubbles[1]).toContain('"updateComponents"'); + }); + it('does not classify plain text messages mentioning "version" as layout snapshots', async () => { const historyMocks: LlmMessage[] = [ { @@ -452,7 +489,7 @@ describe('ChatPanel Gemini Dialogue Panel Integration', () => { it('strips XML/HTML thinking tags and streaming pulse indicators when classifying layout snapshots', async () => { const historyMocks: LlmMessage[] = [ { - role: MessageRole.USER, + role: MessageRole.MODEL, content: `Thinking process...\n{"version": "v0.9", "updateComponents": {"surfaceId": "s1", "components": [{"id":"b1","component":"Button"}]}} ●●●`, }, ]; @@ -464,14 +501,14 @@ describe('ChatPanel Gemini Dialogue Panel Integration', () => { const bubbleTypes = await harness.getBubbleTypes(); expect(bubbles.length).toBe(1); - expect(bubbleTypes[0]).toBe('layout-snapshot'); + expect(bubbleTypes[0]).toBe('model-response'); expect(bubbles[0]).toBe('Received 1 A2UI JSON Components'); }); it('ignores prose text brackets when extracting JSON content for snapshot classification', async () => { const historyMocks: LlmMessage[] = [ { - role: MessageRole.USER, + role: MessageRole.MODEL, content: 'Here is the layout [note]: {"version": "v0.9", "updateComponents": {"surfaceId": "s1", "components": [{"id":"b1","component":"Button"}]}}', }, @@ -484,7 +521,7 @@ describe('ChatPanel Gemini Dialogue Panel Integration', () => { const bubbleTypes = await harness.getBubbleTypes(); expect(bubbles.length).toBe(1); - expect(bubbleTypes[0]).toBe('layout-snapshot'); + expect(bubbleTypes[0]).toBe('model-response'); expect(bubbles[0]).toBe('Received 1 A2UI JSON Components'); }); @@ -699,6 +736,54 @@ describe('ChatPanel Gemini Dialogue Panel Integration', () => { expect(await dialog.getTitleText()).toBe('System Instructions'); }); + it('builds multimodal system instructions when includeScreenshot is enabled or an image attachment is staged', async () => { + const component = fixture.componentInstance; + expect(component.systemPrompt()).toBe('Initial system prompt instructions block'); + + // Enabling includeScreenshot switches systemPrompt() to multimodal + await harness.toggleScreenshot(); + fixture.detectChanges(); + expect(component.systemPrompt()).toBe('multimodal prompt'); + expect(promptFactoryMock.buildSystemPrompt).toHaveBeenCalledWith({ + includeMultimodalGuidelines: true, + }); + + // Disabling includeScreenshot switches systemPrompt() back to text-only + await harness.toggleScreenshot(); + fixture.detectChanges(); + expect(component.systemPrompt()).toBe('Initial system prompt instructions block'); + + // Staging a non-image attachment keeps systemPrompt() text-only + component.attachedFiles.set([ + { + name: 'data.json', + mimeType: 'application/json', + data: 'e30=', + }, + ]); + fixture.detectChanges(); + expect(component.systemPrompt()).toBe('Initial system prompt instructions block'); + + // Staging an image attachment switches systemPrompt() to multimodal + promptFactoryMock.buildSystemPrompt.mockClear(); + component.attachedFiles.set([ + { + name: 'mockup.webp', + mimeType: 'image/webp', + data: 'base64data', + previewUrl: 'data:image/webp;base64,base64data', + }, + ]); + fixture.detectChanges(); + + await harness.clickSystemInstructionsLink(); + fixture.detectChanges(); + + expect(promptFactoryMock.buildSystemPrompt).toHaveBeenCalledWith({ + includeMultimodalGuidelines: true, + }); + }); + it('renders custom instructions link and updates text based on active custom preset', async () => { expect(await harness.hasCustomInstructionsLink()).toBe(true); expect(await harness.getCustomInstructionsLinkText()).toBe('Custom Instructions'); diff --git a/shell/src/app/chat/chat-prompt-factory/chat-prompt-factory.service.spec.ts b/shell/src/app/chat/chat-prompt-factory/chat-prompt-factory.service.spec.ts index a4b090110..41c333f95 100644 --- a/shell/src/app/chat/chat-prompt-factory/chat-prompt-factory.service.spec.ts +++ b/shell/src/app/chat/chat-prompt-factory/chat-prompt-factory.service.spec.ts @@ -18,6 +18,7 @@ import {TestBed} from '@angular/core/testing'; import {ChatPromptFactoryService} from './chat-prompt-factory.service'; import {CatalogManagement} from '../../storage/catalog-management/catalog-management'; import {McpClientManagerService} from '../../mcp/mcp-client-manager.service'; +import {COMMON_TYPES_SCHEMA} from '../../gallery/schema/common-types-schema'; import {LocalStorageInteractions} from '../../storage/local-storage-interactions/local-storage-interactions'; import {LocalStorageKey} from '../../storage/models/local-storage-keys'; @@ -219,4 +220,196 @@ describe('ChatPromptFactoryService', () => { ); }); }); + + describe('Multimodal Guidelines', () => { + beforeEach(() => { + catalogSpy.activeCatalog.mockReturnValue({components: {}}); + }); + + it('omits ## Multimodal & Image-to-UI Guidelines by default in systemPrompt() and buildSystemPrompt() with clean section boundary formatting', () => { + const prompt = service.systemPrompt(); + expect(prompt).not.toContain('## Multimodal & Image-to-UI Guidelines'); + expect(service.buildSystemPrompt()).not.toContain('## Multimodal & Image-to-UI Guidelines'); + expect(service.buildSystemPrompt({includeMultimodalGuidelines: false})).not.toContain( + '## Multimodal & Image-to-UI Guidelines', + ); + expect(prompt).toContain( + 'independently parseable by JSON.parse().\n\n ## Validation & Lifecycle Ordering', + ); + expect(prompt).not.toContain('\n\n\n'); + }); + + it('includes ## Multimodal & Image-to-UI Guidelines when includeMultimodalGuidelines is true with follow-up precedence note and clean boundaries', () => { + const prompt = service.buildSystemPrompt({includeMultimodalGuidelines: true}); + expect(prompt).toContain('## Multimodal & Image-to-UI Guidelines'); + expect(prompt).toContain( + 'explicit user text instructions to add, modify, or remove elements\n take precedence over earlier screenshots and follow ## Editing the Current UI', + ); + expect(prompt).toContain('### 1. Visual Layout, Scope & Sizing Fidelity'); + expect(prompt).toContain('### 6. Image-to-UI Processing Sequence'); + expect(prompt).toContain( + 'independently parseable by JSON.parse().\n\n ## Multimodal & Image-to-UI Guidelines', + ); + expect(prompt).toContain( + '(createSurface -> updateComponents -> updateDataModel).\n\n ## Validation & Lifecycle Ordering', + ); + expect(prompt).not.toContain('\n\n\n'); + }); + }); + + describe('Schema Minification', () => { + it('serializes catalog and COMMON_TYPES_SCHEMA as compact single-line JSON while preserving COMMON_TYPES_SCHEMA descriptions', () => { + const catalog = { + catalogId: 'compact-cat', + components: { + Text: { + type: 'object', + properties: {text: {type: 'string'}}, + }, + }, + }; + catalogSpy.activeCatalog.mockReturnValue(catalog); + + const prompt = service.systemPrompt(); + expect(prompt).toContain( + '{"catalogId":"compact-cat","components":{"Text":{"type":"object","properties":{"text":{"type":"string"}}}}}', + ); + expect(prompt).toContain(JSON.stringify(COMMON_TYPES_SCHEMA)); + expect(prompt).toContain( + 'Attributes to enhance accessibility when using assistive technologies', + ); + }); + + it('preserves transitively referenced $defs (including common_types_* and custom $defs) while pruning unreferenced $defs without mutating input catalog', () => { + const catalog = { + catalogId: 'defs-cat', + components: { + Button: { + type: 'object', + properties: { + label: {$ref: '#/$defs/common_types_DynamicString'}, + variant: {$ref: '#/$defs/CustomVariant'}, + }, + }, + }, + $defs: { + common_types_DynamicString: { + anyOf: [{type: 'string'}, {$ref: '#/$defs/common_types_DataBinding'}], + }, + common_types_DataBinding: {type: 'object'}, + CustomVariant: {type: 'string', enum: ['primary', 'secondary']}, + catalog_Button: {type: 'object', description: 'Unreferenced duplicate component copy'}, + UnusedDef: {type: 'number'}, + }, + }; + catalogSpy.activeCatalog.mockReturnValue(catalog); + + const prompt = service.systemPrompt(); + expect(prompt).toContain('"common_types_DynamicString"'); + expect(prompt).toContain('"common_types_DataBinding"'); + expect(prompt).toContain('"CustomVariant":{"type":"string","enum":["primary","secondary"]}'); + expect(prompt).not.toContain('catalog_Button'); + expect(prompt).not.toContain('UnusedDef'); + // Input catalog object must not be mutated + expect(catalog.$defs.catalog_Button).toBeDefined(); + expect(catalog.$defs.UnusedDef).toEqual({type: 'number'}); + }); + + it('omits $defs when catalog.$defs contains only unreferenced entries', () => { + const unreferencedDefsCatalog = { + catalogId: 'only-unreferenced-defs', + components: {Text: {type: 'object'}}, + $defs: { + catalog_Text: {type: 'object'}, + }, + }; + catalogSpy.activeCatalog.mockReturnValue(unreferencedDefsCatalog); + expect(service.systemPrompt()).toContain( + '{"catalogId":"only-unreferenced-defs","components":{"Text":{"type":"object"}}}', + ); + expect(unreferencedDefsCatalog.$defs.catalog_Text).toEqual({type: 'object'}); + }); + }); + + describe('Condensed Few-Shot Examples', () => { + beforeEach(() => { + catalogSpy.activeCatalog.mockReturnValue({components: {}}); + }); + + it('replaces the verbose vacation_booking example with compact valid JSONL examples covering core A2UI features', () => { + const prompt = service.systemPrompt(); + expect(prompt).not.toContain('vacation_booking'); + expect(prompt).not.toContain('Book Your Dream Vacation'); + + const examplesMatch = prompt.match(/## Examples([\s\S]*?)## Data Binding/); + expect(examplesMatch).not.toBeNull(); + const examplesSection = examplesMatch![1]; + expect(examplesSection.length).toBeLessThanOrEqual(1500); + + const jsonlBlocks = Array.from(examplesSection.matchAll(/```jsonl\n([\s\S]*?)```/g)).map(m => + m[1].trim(), + ); + expect(jsonlBlocks.length).toBeGreaterThanOrEqual(2); + + for (const block of jsonlBlocks) { + const rawLines = block.split('\n').map(line => line.trim()); + for (const rawLine of rawLines) { + expect(rawLine).toBe(JSON.stringify(JSON.parse(rawLine))); + } + + const blockMessages = rawLines.map(line => JSON.parse(line)); + const createMsg = blockMessages.find(m => m.createSurface); + const updateCompMsg = blockMessages.find(m => m.updateComponents); + const updateDataMsg = blockMessages.find(m => m.updateDataModel); + expect(createMsg).toBeDefined(); + expect(updateCompMsg).toBeDefined(); + expect(updateDataMsg).toBeDefined(); + + const surfaceId = createMsg.createSurface.surfaceId; + expect(updateCompMsg.updateComponents.surfaceId).toBe(surfaceId); + expect(updateDataMsg.updateDataModel.surfaceId).toBe(surfaceId); + + const components: Array<{ + id: string; + child?: string; + children?: string[] | {componentId?: string}; + }> = updateCompMsg.updateComponents.components; + const componentIds = new Set(components.map(c => c.id)); + expect(componentIds.has('root')).toBe(true); + + for (const comp of components) { + if (typeof comp.child === 'string') { + expect(componentIds.has(comp.child)).toBe(true); + } + if (Array.isArray(comp.children)) { + for (const childId of comp.children) { + expect(componentIds.has(childId)).toBe(true); + } + } else if (comp.children && typeof comp.children.componentId === 'string') { + expect(componentIds.has(comp.children.componentId)).toBe(true); + } + } + } + + const parsedLines = jsonlBlocks + .flatMap(block => block.split('\n')) + .map(line => JSON.parse(line.trim())); + + expect(parsedLines.some(msg => msg.version === 'v0.9' && msg.createSurface)).toBe(true); + expect(parsedLines.some(msg => msg.version === 'v0.9' && msg.updateComponents)).toBe(true); + expect(parsedLines.some(msg => msg.version === 'v0.9' && msg.updateDataModel)).toBe(true); + + const serializedExamples = JSON.stringify(parsedLines); + // JSON Pointer data binding + expect(serializedExamples).toContain('"path":"/'); + // Button event action with context binding + expect(serializedExamples).toContain( + '"action":{"event":{"name":"submitForm","context":{"name":{"path":"/name"}}}}', + ); + // Dynamic list template + expect(serializedExamples).toContain( + '"children":{"componentId":"item_template","path":"/items"}', + ); + }); + }); }); diff --git a/shell/src/app/chat/chat-prompt-factory/chat-prompt-factory.service.ts b/shell/src/app/chat/chat-prompt-factory/chat-prompt-factory.service.ts index 502aaa77d..ecdab1885 100644 --- a/shell/src/app/chat/chat-prompt-factory/chat-prompt-factory.service.ts +++ b/shell/src/app/chat/chat-prompt-factory/chat-prompt-factory.service.ts @@ -16,7 +16,7 @@ import {Injectable, computed, inject, signal} from '@angular/core'; import {CatalogManagement} from '../../storage/catalog-management/catalog-management'; -import {formatJson} from '../../utils/json'; +import {Catalog} from '../../storage/models/catalog-storage.model'; import {COMMON_TYPES_SCHEMA} from '../../gallery/schema/common-types-schema'; import { McpClientManagerService, @@ -43,6 +43,8 @@ export declare interface CustomInstructionsState { activePresetId: string | null; } +const MINIFIED_COMMON_TYPES_SCHEMA = JSON.stringify(COMMON_TYPES_SCHEMA); + /** * Constructs dynamic system prompts based on the provided LLM intent and active catalog states. */ @@ -64,7 +66,21 @@ export class ChatPromptFactoryService { readonly customInstructions = computed(() => this.activePreset()?.content ?? ''); readonly hasCustomInstructions = computed(() => this.customInstructions().trim().length > 0); - readonly systemPrompt = computed(() => { + private readonly textOnlySystemPrompt = computed(() => this.composeSystemPrompt(false)); + private readonly multimodalSystemPrompt = computed(() => this.composeSystemPrompt(true)); + + readonly systemPrompt = this.textOnlySystemPrompt; + + /** + * Builds the system prompt with optional multimodal image-to-UI guidelines. + */ + buildSystemPrompt(options?: {includeMultimodalGuidelines?: boolean}): string { + return options?.includeMultimodalGuidelines + ? this.multimodalSystemPrompt() + : this.textOnlySystemPrompt(); + } + + private composeSystemPrompt(includeMultimodalGuidelines: boolean): string { const catalog = this.catalogManagement.activeCatalog(); const mcpSupported = this.mcpManager.doesCatalogSupportMcp(catalog); const activeServers = mcpSupported ? this.mcpManager.getActiveServersWithTools() : []; @@ -89,10 +105,14 @@ export class ChatPromptFactoryService { } return ( - (this.generateSystemPrompt(formatJson(catalog)) + mcpInstructions).trimEnd() + - customPromptSuffix + ( + this.generateSystemPrompt( + this.serializeCatalogForPrompt(catalog), + includeMultimodalGuidelines, + ) + mcpInstructions + ).trimEnd() + customPromptSuffix ); - }); + } /** * Updates the custom instruction state and persists it to local storage. @@ -215,78 +235,67 @@ export class ChatPromptFactoryService { `; } - private generateSystemPrompt(catalog: string): string { - return ` - # A2UI Generation Expert - - ## Role - You are an expert A2UI generation assistant. Your role is to translate user - requests—whether provided as text instructions, UI wireframes, screenshots, - or mockup images—into valid A2UI v0.9 interactive user interfaces. - - ## Catalog Allowlist & Component Rules - - You MUST strictly enforce the following rules regarding component selection - and schema compliance: - 1. **Strict Component Allowlist**: You MUST use ONLY the component types - defined as keys in the "components" map of the active catalog schema - provided below. - 2. **No Hallucinated Component Names**: Never invent, guess, or mix - component names from other libraries or catalogs. For example, if only - "Column", "Row", "Text", and "Button" are present in the active catalog - schema below, emitting "MaterialColumn", "MaterialText", or "Div" is - strictly INVALID. - 3. **No Hallucinated Properties**: Include ONLY properties explicitly - defined in the JSON Schema for that specific component type in the - active catalog. Do NOT emit unauthorized keys (e.g., "rules", "mock*", - or unsupported CSS/styling parameters). - - ### Active Catalog Schema (Mandatory Allowlist) - \`\`\`json - ${catalog} - \`\`\` - - ### Common Schema Types - Common structural types referenced by $ref in the catalog schema (e.g., - DataBinding, Action, Event, DynamicString, etc.) are defined here: - \`\`\`json - ${formatJson(COMMON_TYPES_SCHEMA)} - \`\`\` - - ## Editing the Current UI + /** + * Serializes the catalog as compact JSON, pruning `$defs` entries that are not + * transitively referenced by `components` or `functions` (such as bundled + * duplicate `catalog_*` definitions). + */ + private serializeCatalogForPrompt(catalog: Catalog): string { + if (!catalog.$defs || typeof catalog.$defs !== 'object' || Array.isArray(catalog.$defs)) { + return JSON.stringify(catalog); + } + const defs = catalog.$defs; + const referenced = new Set(); + this.collectLocalDefRefs( + {components: catalog.components, functions: catalog.functions}, + referenced, + ); + for (const currentKey of referenced) { + const defNode = defs[currentKey]; + if (defNode !== undefined) { + this.collectLocalDefRefs(defNode, referenced); + } + } + const filteredDefs = Object.fromEntries( + Object.entries(defs).filter(([key]) => referenced.has(key)), + ); + const prunedCatalog: Catalog = {...catalog}; + if (Object.keys(filteredDefs).length > 0) { + prunedCatalog.$defs = filteredDefs; + } else { + delete prunedCatalog.$defs; + } + return JSON.stringify(prunedCatalog); + } - When a current editor A2UI snapshot is provided, it is the authoritative UI - being edited. Use its actual surface IDs, component IDs, and data bindings; - never borrow IDs or paths from the illustrative examples below. For literal - Text values, emit updateComponents with the edited component and its existing - properties. Use updateDataModel only for values bound to that surface's model. - Preserve unrelated components and update all labels affected by the request - (for example, a destination's airport code, city label, and route heading). - For edits, emit updates to existing surfaces. For a replacement UI, emit a - complete document beginning with createSurface before its updates. + private collectLocalDefRefs(node: unknown, out: Set): void { + if (Array.isArray(node)) { + for (const item of node) { + this.collectLocalDefRefs(item, out); + } + return; + } + if (node !== null && typeof node === 'object') { + for (const [key, val] of Object.entries(node)) { + if (key === '$ref' && typeof val === 'string' && val.startsWith('#/$defs/')) { + out.add(val.slice('#/$defs/'.length)); + } else { + this.collectLocalDefRefs(val, out); + } + } + } + } - ## Output Format: Strict A2UI JSON Lines (JSONL) - - Your output MUST be valid **A2UI JSON Lines (JSONL)**: - 1. **One JSON Object Per Line**: Each A2UI message MUST be formatted as a - single, valid JSON object on its own line, terminated by a newline - character (\\n). Do NOT pretty-print or split a single JSON object across - multiple lines. - 2. **Required Version & Command**: Every message object MUST include - "version": "v0.9" at the top level and specify exactly one A2UI - command: "createSurface", "updateComponents", "updateDataModel", or - "deleteSurface". - 3. **No Markdown or Preamble**: Output ONLY raw JSON Lines. Do NOT wrap - your response in markdown code fences (such as \`\`\`jsonl or \`\`\`). Do - NOT include any conversational text, greetings, explanations, - scratchpad analysis, or summary before or after the JSON Lines. - 4. **Direct Parseability**: Every line in your response MUST be - independently parseable by JSON.parse(). + private generateSystemPrompt(catalog: string, includeMultimodalGuidelines = false): string { + const multimodalGuidelinesSection = includeMultimodalGuidelines + ? ` ## Multimodal & Image-to-UI Guidelines When an image, wireframe, mockup, or UI screenshot is provided by the - user, adhere strictly to these visual translation principles: + user, adhere strictly to these visual translation principles (on follow-up + turns, explicit user text instructions to add, modify, or remove elements + take precedence over earlier screenshots and follow ## Editing the Current UI): ### 1. Visual Layout, Scope & Sizing Fidelity * **Root Container Bounding**: The root component ("id": "root") MUST match @@ -415,7 +424,75 @@ export class ChatPromptFactoryService { catalog component types with exact icon names/SVGs, full-width properties, and JSON Pointer paths. 4. **Emit JSONL Messages**: Output the single-line JSONL messages in - strict sequence (createSurface -> updateComponents -> updateDataModel). + strict sequence (createSurface -> updateComponents -> updateDataModel).` + : ''; + + return ` + # A2UI Generation Expert + + ## Role + You are an expert A2UI generation assistant. Your role is to translate user + requests—whether provided as text instructions, UI wireframes, screenshots, + or mockup images—into valid A2UI v0.9 interactive user interfaces. + + ## Catalog Allowlist & Component Rules + + You MUST strictly enforce the following rules regarding component selection + and schema compliance: + 1. **Strict Component Allowlist**: You MUST use ONLY the component types + defined as keys in the "components" map of the active catalog schema + provided below. + 2. **No Hallucinated Component Names**: Never invent, guess, or mix + component names from other libraries or catalogs. For example, if only + "Column", "Row", "Text", and "Button" are present in the active catalog + schema below, emitting "MaterialColumn", "MaterialText", or "Div" is + strictly INVALID. + 3. **No Hallucinated Properties**: Include ONLY properties explicitly + defined in the JSON Schema for that specific component type in the + active catalog. Do NOT emit unauthorized keys (e.g., "rules", "mock*", + or unsupported CSS/styling parameters). + + ### Active Catalog Schema (Mandatory Allowlist) + \`\`\`json + ${catalog} + \`\`\` + + ### Common Schema Types + Common structural types referenced by $ref in the catalog schema (e.g., + DataBinding, Action, Event, DynamicString, etc.) are defined here: + \`\`\`json + ${MINIFIED_COMMON_TYPES_SCHEMA} + \`\`\` + + ## Editing the Current UI + + When a current editor A2UI snapshot is provided, it is the authoritative UI + being edited. Use its actual surface IDs, component IDs, and data bindings; + never borrow IDs or paths from the illustrative examples below. For literal + Text values, emit updateComponents with the edited component and its existing + properties. Use updateDataModel only for values bound to that surface's model. + Preserve unrelated components and update all labels affected by the request + (for example, a destination's airport code, city label, and route heading). + For edits, emit updates to existing surfaces. For a replacement UI, emit a + complete document beginning with createSurface before its updates. + + ## Output Format: Strict A2UI JSON Lines (JSONL) + + Your output MUST be valid **A2UI JSON Lines (JSONL)**: + 1. **One JSON Object Per Line**: Each A2UI message MUST be formatted as a + single, valid JSON object on its own line, terminated by a newline + character (\\n). Do NOT pretty-print or split a single JSON object across + multiple lines. + 2. **Required Version & Command**: Every message object MUST include + "version": "v0.9" at the top level and specify exactly one A2UI + command: "createSurface", "updateComponents", "updateDataModel", or + "deleteSurface". + 3. **No Markdown or Preamble**: Output ONLY raw JSON Lines. Do NOT wrap + your response in markdown code fences (such as \`\`\`jsonl or \`\`\`). Do + NOT include any conversational text, greetings, explanations, + scratchpad analysis, or summary before or after the JSON Lines. + 4. **Direct Parseability**: Every line in your response MUST be + independently parseable by JSON.parse().${multimodalGuidelinesSection} ## Validation & Lifecycle Ordering @@ -430,7 +507,9 @@ export class ChatPromptFactoryService { You must provide a flat list of components. One component MUST have an id of "root". 3. **updateDataModel**: Used to define or update data values that the - components bind to. + components bind to (optionally specifying a JSON Pointer "path", e.g., + {"version":"v0.9","updateDataModel":{"surfaceId":"main","path":"/name","value":"Bob"}}, + for targeted partial updates). 4. **deleteSurface**: Signals the client to destroy the surface. Typical sequence: createSurface -> updateComponents -> updateDataModel @@ -440,42 +519,31 @@ export class ChatPromptFactoryService { ## Examples - **IMPORTANT**: The component names used in the examples below (Column, Text, - TextField, ChoicePicker, Button, etc.) are for structural illustration. - You MUST replace them with exact component names from the active catalog - schema provided above. In addition, code fences (\`\`\`jsonl) are shown - below for documentation readability only; do NOT include code fences in - your actual JSONL output. - - * **Simple Example**: A basic column with text: - \`\`\`jsonl - {"version": "v0.9", "createSurface": {"surfaceId": "main", "catalogId": "https://a2ui.org/specification/v0_9/material_catalog.json"}} - {"version": "v0.9", "updateComponents": {"surfaceId": "main", "components": [{"id": "root", "component": "MaterialColumn", "children": ["header", "content"]}, {"id": "header", "component": "MaterialText", "text": "Welcome"}, {"id": "content", "component": "MaterialText", "text": {"path": "/message"}}]}} - {"version": "v0.9", "updateDataModel": {"surfaceId": "main", "path": "/message", "value": "Hello, world!"}} - \`\`\` + **IMPORTANT**: Catalog IDs, component names, and component properties (e.g., + child vs label) below are illustrative; strictly follow the active catalog + schema above and omit \`\`\`jsonl fences in output. - * **Complex Form Example**: A vacation booking form demonstrating advanced - Material form controls (\`MaterialDatepicker\`, \`MaterialSelect\`, - \`MaterialSlideToggle\`) and buttons using the modernized Material catalog: + * **Form & Action Example**: \`\`\`jsonl - {"version": "v0.9", "createSurface": {"surfaceId": "vacation_booking", "catalogId": "https://a2ui.org/specification/v0_9/material_catalog.json"}} - {"version": "v0.9", "updateComponents": {"surfaceId": "vacation_booking", "components": [{"id": "root", "component": "MaterialColumn", "children": ["title", "destination_input", "checkin_datepicker", "checkout_datepicker", "room_type_select", "passenger_select", "flexible_dates_toggle", "search_button"]}, {"id": "title", "component": "MaterialText", "text": {"path": "/title_label"}, "usageHint": "h1"}, {"id": "destination_input", "component": "MaterialInput", "label": {"path": "/destination_label"}, "value": {"path": "/destination_value"}}, {"id": "checkin_datepicker", "component": "MaterialDatepicker", "label": {"path": "/checkin_label"}, "value": {"path": "/checkin_value"}}, {"id": "checkout_datepicker", "component": "MaterialDatepicker", "label": {"path": "/checkout_label"}, "value": {"path": "/checkout_value"}}, {"id": "room_type_select", "component": "MaterialSelect", "label": {"path": "/room_type_label"}, "value": {"path": "/room_type_value"}, "options": [{"label": "Standard Room", "value": "standard"}, {"label": "Deluxe Suite", "value": "deluxe"}]}, {"id": "passenger_select", "component": "MaterialSelect", "label": {"path": "/passenger_label"}, "value": {"path": "/passenger_value"}, "options": [{"label": "1 Passenger", "value": "1"}, {"label": "2 Passengers", "value": "2"}, {"label": "3+ Passengers", "value": "3"}]}, {"id": "flexible_dates_toggle", "component": "MaterialSlideToggle", "label": {"path": "/flexible_dates_label"}, "checked": {"path": "/flexible_dates_checked"}, "color": "primary"}, {"id": "search_button", "component": "MaterialButton", "label": {"path": "/search_label"}, "action": {"event": {"name": "searchVacation"}}}]}} - {"version": "v0.9", "updateDataModel": {"surfaceId": "vacation_booking", "value": {"title_label": "Book Your Dream Vacation", "destination_label": "Destination", "destination_value": "Hawaii", "checkin_label": "Check-in Date", "checkin_value": "2026-07-01", "checkout_label": "Check-out Date", "checkout_value": "2026-07-14", "room_type_label": "Room Type", "room_type_value": "standard", "passenger_label": "Passengers", "passenger_value": "2", "flexible_dates_label": "Flexible Dates (+/- 3 days)", "flexible_dates_checked": true, "search_label": "Search Flights & Hotels"}}} + {"version":"v0.9","createSurface":{"surfaceId":"main","catalogId":"https://a2ui.org/specification/v0_9/material_catalog.json"}} + {"version":"v0.9","updateComponents":{"surfaceId":"main","components":[{"id":"root","component":"MaterialColumn","children":["name_in","btn"]},{"id":"name_in","component":"MaterialInput","label":{"path":"/label"},"value":{"path":"/name"}},{"id":"btn","component":"MaterialButton","label":{"path":"/submitLabel"},"action":{"event":{"name":"submitForm","context":{"name":{"path":"/name"}}}}}]}} + {"version":"v0.9","updateDataModel":{"surfaceId":"main","value":{"label":"Name","name":"Alice","submitLabel":"Submit"}}} \`\`\` - * **Dynamic List Example**: An example using templates to render a list of - items. + * **Dynamic List Example**: \`\`\`jsonl - {"version": "v0.9", "createSurface": {"surfaceId": "dynamic_list_demo", "catalogId": "https://a2ui.org/specification/v0_9/material_catalog.json"}} - {"version": "v0.9", "updateComponents": {"surfaceId": "dynamic_list_demo", "components": [{"id": "root", "component": "MaterialColumn", "children": ["title", "list_container"]}, {"id": "title", "component": "MaterialText", "text": "Dynamic List Demo"}, {"id": "list_container", "component": "MaterialColumn", "children": {"componentId": "item_template", "path": "/items"}}, {"id": "item_template", "component": "MaterialText", "text": {"path": "text"}}]}} - {"version": "v0.9", "updateDataModel": {"surfaceId": "dynamic_list_demo", "value": {"items": [{"text": "Item One"}, {"text": "Item Two"}]}}} + {"version":"v0.9","createSurface":{"surfaceId":"list","catalogId":"https://a2ui.org/specification/v0_9/material_catalog.json"}} + {"version":"v0.9","updateComponents":{"surfaceId":"list","components":[{"id":"root","component":"MaterialColumn","children":{"componentId":"item_template","path":"/items"}},{"id":"item_template","component":"MaterialText","text":{"path":"text"}}]}} + {"version":"v0.9","updateDataModel":{"surfaceId":"list","value":{"items":[{"text":"One"},{"text":"Two"}]}}} \`\`\` ## Data Binding Every component property value MUST come from the data model (with minor exceptions for static primitives). - When referencing data in the data model, you MUST use valid JSON Pointer - syntax starting with /. + When referencing root data in the data model, use valid JSON Pointer syntax + starting with / (e.g., {"path":"/name"}). Inside item templates bound via + children: {"componentId": "...", "path": "/items"}, use relative property + paths without a leading slash (e.g., {"path":"text"}). ## Actions and Context diff --git a/shell/src/app/chat/copilotkit-chat-panel/copilotkit-chat-panel.spec.ts b/shell/src/app/chat/copilotkit-chat-panel/copilotkit-chat-panel.spec.ts index d1ad4d75c..9e8b93fcb 100644 --- a/shell/src/app/chat/copilotkit-chat-panel/copilotkit-chat-panel.spec.ts +++ b/shell/src/app/chat/copilotkit-chat-panel/copilotkit-chat-panel.spec.ts @@ -113,6 +113,9 @@ class MockChatCoordinator { class MockChatPromptFactoryService { readonly systemPrompt = signal('Initial system prompt instructions block'); + buildSystemPrompt = vi.fn((opts?: {includeMultimodalGuidelines?: boolean}) => + opts?.includeMultimodalGuidelines ? 'multimodal prompt' : this.systemPrompt(), + ); readonly customInstructionsState = signal({ presets: [], activePresetId: null, diff --git a/shell/src/app/chat/file-ingestion/file-ingestion.service.spec.ts b/shell/src/app/chat/file-ingestion/file-ingestion.service.spec.ts index e8933f48f..b44ae615c 100644 --- a/shell/src/app/chat/file-ingestion/file-ingestion.service.spec.ts +++ b/shell/src/app/chat/file-ingestion/file-ingestion.service.spec.ts @@ -45,4 +45,22 @@ describe('FileIngestionService', () => { expect(result.data).toBe(btoa('blobdata')); expect(result.previewUrl).toContain('data:image/png;base64,'); }); + + it('infers image MIME type and constructs normalized previewUrl from filename extension when File.type is empty or application/octet-stream', async () => { + const webpFile = new File(['webpdata'], 'wireframe.webp', {type: ''}); + const webpResult = await service.readFileAsAttachment(webpFile); + + expect(webpResult.name).toBe('wireframe.webp'); + expect(webpResult.mimeType).toBe('image/webp'); + expect(webpResult.data).toBe(btoa('webpdata')); + expect(webpResult.previewUrl).toBe(`data:image/webp;base64,${btoa('webpdata')}`); + + const svgFile = new File([''], 'icon.svg', {type: 'application/octet-stream'}); + const svgResult = await service.readFileAsAttachment(svgFile); + + expect(svgResult.name).toBe('icon.svg'); + expect(svgResult.mimeType).toBe('image/svg+xml'); + expect(svgResult.data).toBe(btoa('')); + expect(svgResult.previewUrl).toBe(`data:image/svg+xml;base64,${btoa('')}`); + }); }); diff --git a/shell/src/app/chat/file-ingestion/file-ingestion.service.ts b/shell/src/app/chat/file-ingestion/file-ingestion.service.ts index b63cab58d..806627e13 100644 --- a/shell/src/app/chat/file-ingestion/file-ingestion.service.ts +++ b/shell/src/app/chat/file-ingestion/file-ingestion.service.ts @@ -15,6 +15,7 @@ */ import {Injectable} from '@angular/core'; +import {inferMimeType, isImageAttachment} from '../a2a/a2a-attachments'; import {Attachment} from '../llm-client/llm-client'; /** @@ -51,11 +52,14 @@ export class FileIngestionService { return; } const base64Data = result.substring(commaIndex + 1); + const mimeType = inferMimeType(file.name, file.type); resolve({ name: file.name, - mimeType: file.type || 'application/octet-stream', + mimeType, data: base64Data, - previewUrl: file.type.startsWith('image/') ? result : undefined, + previewUrl: isImageAttachment(mimeType, file.name) + ? `data:${mimeType};base64,${base64Data}` + : undefined, }); }; reader.onerror = err => reject(err); diff --git a/shell/src/app/chat/llm-client/llm-client.ts b/shell/src/app/chat/llm-client/llm-client.ts index eef0742fb..ad267daa8 100644 --- a/shell/src/app/chat/llm-client/llm-client.ts +++ b/shell/src/app/chat/llm-client/llm-client.ts @@ -88,6 +88,9 @@ export declare interface LlmMessage { /** Indicates whether the message represents an established state snapshot rather than an incremental delta. */ readonly isSnapshot?: boolean; + /** Indicates whether the model turn was stopped/cancelled by the user before completion. */ + readonly isCancelled?: boolean; + /** The total count of recognized A2UI components contained in this message, or null/undefined if uncounted. */ readonly componentCount?: number | null; } @@ -116,6 +119,7 @@ export interface LlmResponse { } export const CANCEL_ERROR_NAME = 'CancelError'; +export const STOPPED_RESPONSE_MESSAGE = '*You stopped this response.*'; /** * Represents dynamic, real-time incremental tokens compiled asynchronously. diff --git a/shell/src/app/chat/state-sync/state-sync.spec.ts b/shell/src/app/chat/state-sync/state-sync.spec.ts index f38504d41..0f0b0b52e 100644 --- a/shell/src/app/chat/state-sync/state-sync.spec.ts +++ b/shell/src/app/chat/state-sync/state-sync.spec.ts @@ -204,7 +204,8 @@ describe('StateSync Autosave Draft Integrations', () => { expect(chatStateMock.setChatHistory).toHaveBeenCalledWith([ { role: MessageRole.USER, - content: '[\n {\n "version": "v0.9"\n }\n]', + content: '[{"version":"v0.9"}]', + isSnapshot: true, }, ]); }); @@ -214,7 +215,7 @@ describe('StateSync Autosave Draft Integrations', () => { chatStateMock.setChatHistory([ { role: MessageRole.USER, - content: '[\n {\n "version": "v0.9",\n "orig": true\n }\n]', + content: '[{"version":"v0.9","orig":true}]', }, ]); @@ -227,7 +228,8 @@ describe('StateSync Autosave Draft Integrations', () => { expect(chatStateMock.setChatHistory).toHaveBeenCalledWith([ { role: MessageRole.USER, - content: '[\n {\n "version": "v0.9",\n "updated": true\n }\n]', + content: '[{"version":"v0.9","updated":true}]', + isSnapshot: true, }, ]); expect(chatStateMock.updateChatHistory).not.toHaveBeenCalled(); @@ -237,7 +239,7 @@ describe('StateSync Autosave Draft Integrations', () => { chatStateMock.setChatHistory([ { role: MessageRole.USER, - content: '[\n {\n "version": "v0.9"\n }\n]', + content: '[{"version":"v0.9"}]', }, { role: MessageRole.MODEL, @@ -256,7 +258,7 @@ describe('StateSync Autosave Draft Integrations', () => { const initialMockHistory = [ { role: MessageRole.USER, - content: '[\n {\n "version": "v0.9"\n }\n]', + content: '[{"version":"v0.9"}]', }, { role: MessageRole.MODEL, @@ -268,7 +270,8 @@ describe('StateSync Autosave Draft Integrations', () => { expect(updated).toHaveLength(3); expect(updated[2]).toEqual({ role: MessageRole.USER, - content: '[\n {\n "version": "v0.9",\n "post-turn": true\n }\n]', + content: '[{"version":"v0.9","post-turn":true}]', + isSnapshot: true, }); }); @@ -303,7 +306,8 @@ describe('StateSync Autosave Draft Integrations', () => { }); expect(updated[1]).toEqual({ role: MessageRole.USER, - content: '[\n {\n "version": "v0.9",\n "components": []\n }\n]', + content: '[{"version":"v0.9","components":[]}]', + isSnapshot: true, }); }); @@ -410,11 +414,11 @@ describe('StateSync Autosave Draft Integrations', () => { const syncedContent = syncCall[0].content; // Verify component presence and layout structure integrity - expect(syncedContent).toContain('"id": "root"'); - expect(syncedContent).toContain('"id": "c-1"'); + expect(syncedContent).toContain('"id":"root"'); + expect(syncedContent).toContain('"id":"c-1"'); // Verify that 'mock_rules_container' component itself is stripped - expect(syncedContent).not.toContain('"id": "mock_rules_container"'); + expect(syncedContent).not.toContain('"id":"mock_rules_container"'); // Verify that reference to 'mock_rules_container' in root's children array is pruned expect(syncedContent).toContain('"c-1"'); expect(syncedContent).not.toContain('mock_rules_container'); @@ -436,7 +440,7 @@ describe('StateSync Autosave Draft Integrations', () => { const syncCall = chatStateMock.setChatHistory.mock.calls[0][0]; const syncedContent = syncCall[0].content; - expect(syncedContent).toContain('"id": "c-1"'); + expect(syncedContent).toContain('"id":"c-1"'); expect(syncedContent).toContain('"invalid-string-element"'); }); @@ -472,7 +476,7 @@ describe('StateSync Autosave Draft Integrations', () => { const syncCall = chatStateMock.setChatHistory.mock.calls[0][0]; const syncedContent = syncCall[0].content; - expect(syncedContent).toContain('"id": "c-1"'); + expect(syncedContent).toContain('"id":"c-1"'); }); it('sanitizes objects inside component array properties', () => { @@ -491,7 +495,7 @@ describe('StateSync Autosave Draft Integrations', () => { const syncCall = chatStateMock.setChatHistory.mock.calls[0][0]; const syncedContent = syncCall[0].content; - expect(syncedContent).toContain('"label": "L1"'); + expect(syncedContent).toContain('"label":"L1"'); expect(syncedContent).not.toContain('mockProp'); }); @@ -532,7 +536,7 @@ describe('StateSync Autosave Draft Integrations', () => { const initialSnapshot = [ { role: MessageRole.USER, - content: '[\n {\n "version": "v0.9"\n }\n]', + content: '[{"version":"v0.9"}]', }, ]; chatStateMock.setChatHistory(initialSnapshot); @@ -564,16 +568,114 @@ describe('StateSync Autosave Draft Integrations', () => { expect(syncedContent).toContain('"primitive-element"'); }); - it('returns empty array when all layout lines are sanitized to null', () => { + it('discards layout when all layout lines are sanitized to null without syncing empty array to history', () => { service.updateDraft('[{"registerMockRules": true}]'); TestBed.tick(); vi.advanceTimersByTime(300); - const syncCall = chatStateMock.setChatHistory.mock.calls[0][0]; - const syncedContent = syncCall[0].content; + expect(chatStateMock.setChatHistory).not.toHaveBeenCalled(); + expect(chatStateMock.updateChatHistory).not.toHaveBeenCalled(); + }); + + it('discards empty JSON array [] without syncing to history', () => { + service.updateDraft('[]'); + TestBed.tick(); + + vi.advanceTimersByTime(300); + + expect(chatStateMock.setChatHistory).not.toHaveBeenCalled(); + expect(chatStateMock.updateChatHistory).not.toHaveBeenCalled(); + }); + + it('compacts multi-line formatted JSON in chatHistory while preserving formatting in activeDraft', () => { + const prettyDraft = + '[\n {\n "version": "v0.9",\n "createSurface": {\n "surfaceId": "s1"\n }\n }\n]'; + service.updateDraft(prettyDraft); + TestBed.tick(); + vi.advanceTimersByTime(300); + + expect(service.activeDraft()).toBe(prettyDraft); + expect(service.activeDraft()).toContain('\n'); + + const syncedContent = chatStateMock.chatHistory()[0].content; + expect(syncedContent).toBe('[{"version":"v0.9","createSurface":{"surfaceId":"s1"}}]'); + expect(syncedContent).not.toContain('\n'); + }); + + it('deduplicates whitespace-only editor changes via lastSynchronizedLayout', () => { + service.updateDraft('[{"version":"v0.9","createSurface":{"surfaceId":"s1"}}]'); + TestBed.tick(); + vi.advanceTimersByTime(300); + expect(chatStateMock.setChatHistory).toHaveBeenCalledTimes(1); + + chatStateMock.setChatHistory.mockClear(); + chatStateMock.updateChatHistory.mockClear(); + + service.updateDraft( + '[\n {\n "version": "v0.9",\n "createSurface": {\n "surfaceId": "s1"\n }\n }\n]', + ); + TestBed.tick(); + vi.advanceTimersByTime(300); - expect(syncedContent).toBe('[]'); + expect(chatStateMock.setChatHistory).not.toHaveBeenCalled(); + expect(chatStateMock.updateChatHistory).not.toHaveBeenCalled(); + }); + + it('does not append a duplicate USER snapshot when syncActiveDraftToHistory runs after commitLayoutFromLlm without user edits', () => { + const prettyLlmLayout = formatJson([ + {version: 'v0.9', createSurface: {surfaceId: 's1', catalogId: 'basic'}}, + ]); + chatStateMock.setChatHistory([ + {role: MessageRole.USER, content: 'Create a surface', promptId: 'p1'}, + { + role: MessageRole.MODEL, + content: JSON.stringify([ + {version: 'v0.9', createSurface: {surfaceId: 's1', catalogId: 'basic'}}, + ]), + isSnapshot: true, + }, + ]); + chatStateMock.setChatHistory.mockClear(); + chatStateMock.updateChatHistory.mockClear(); + + service.commitLayoutFromLlm(prettyLlmLayout); + service.syncActiveDraftToHistory(); + + expect(chatStateMock.setChatHistory).not.toHaveBeenCalled(); + expect(chatStateMock.updateChatHistory).not.toHaveBeenCalled(); + expect(chatStateMock.chatHistory()).toHaveLength(2); + }); + + it('preserves user prompts starting with [ instead of overwriting them in-place', () => { + chatStateMock.setChatHistory([ + { + role: MessageRole.USER, + content: '[WIP] please render a login card', + promptId: 'p1', + }, + ]); + chatStateMock.setChatHistory.mockClear(); + chatStateMock.updateChatHistory.mockClear(); + + service.updateDraft('[{"version": "v0.9", "createSurface": {"surfaceId": "s1"}}]'); + TestBed.tick(); + vi.advanceTimersByTime(300); + + expect(chatStateMock.setChatHistory).not.toHaveBeenCalled(); + expect(chatStateMock.updateChatHistory).toHaveBeenCalledTimes(1); + expect(chatStateMock.chatHistory()).toEqual([ + { + role: MessageRole.USER, + content: '[WIP] please render a login card', + promptId: 'p1', + }, + { + role: MessageRole.USER, + content: '[{"version":"v0.9","createSurface":{"surfaceId":"s1"}}]', + isSnapshot: true, + }, + ]); }); it('handles updateComponents without components array during sanitization', () => { @@ -588,7 +690,7 @@ describe('StateSync Autosave Draft Integrations', () => { const syncCall = chatStateMock.setChatHistory.mock.calls[0][0]; const syncedContent = syncCall[0].content; - expect(syncedContent).toContain('"components": "not-an-array"'); + expect(syncedContent).toContain('"components":"not-an-array"'); }); }); @@ -807,7 +909,7 @@ describe('StateSync Autosave Draft Integrations', () => { }); it('populates blank createSurface payload on resetToBlankDraft, synchronously syncs to chatHistory at 0ms, and increments sessionResetNonce', () => { - const expectedBlankSurface = formatJson([ + const blankSurfaceItems = [ { version: 'v0.9', createSurface: { @@ -816,7 +918,9 @@ describe('StateSync Autosave Draft Integrations', () => { sendDataModel: true, }, }, - ]); + ]; + const expectedBlankSurface = formatJson(blankSurfaceItems); + const expectedCompactBlankSurface = JSON.stringify(blankSurfaceItems); chatStateMock.setChatHistory.mockClear(); expect(service.sessionResetNonce()).toBe(0); @@ -831,7 +935,8 @@ describe('StateSync Autosave Draft Integrations', () => { expect(chatStateMock.setChatHistory).toHaveBeenCalledWith([ { role: MessageRole.USER, - content: expectedBlankSurface, + content: expectedCompactBlankSurface, + isSnapshot: true, }, ]); @@ -881,7 +986,7 @@ describe('StateSync Autosave Draft Integrations', () => { }); it('cancels pending <300ms updateDraft timer on resetToBlankDraft, flushDraft, and commitLayoutFromLlm, and does not emit duplicate layout bubble at 300ms', () => { - const expectedBlankSurface = formatJson([ + const expectedCompactBlankSurface = JSON.stringify([ { version: 'v0.9', createSurface: { @@ -905,7 +1010,8 @@ describe('StateSync Autosave Draft Integrations', () => { expect(chatStateMock.chatHistory()).toEqual([ { role: MessageRole.USER, - content: expectedBlankSurface, + content: expectedCompactBlankSurface, + isSnapshot: true, }, ]); @@ -950,7 +1056,7 @@ describe('StateSync Autosave Draft Integrations', () => { }); it('synchronously re-populates chatHistory across repeated consecutive resetToBlankDraft calls and after commitLayoutFromLlm', () => { - const expectedBlankSurface = formatJson([ + const blankSurfaceItems = [ { version: 'v0.9', createSurface: { @@ -959,14 +1065,16 @@ describe('StateSync Autosave Draft Integrations', () => { sendDataModel: true, }, }, - ]); + ]; + const expectedBlankSurface = formatJson(blankSurfaceItems); + const expectedCompactBlankSurface = JSON.stringify(blankSurfaceItems); // First reset chatStateMock.setChatHistory([]); service.resetToBlankDraft(); expect(service.sessionResetNonce()).toBe(1); expect(chatStateMock.chatHistory()).toEqual([ - {role: MessageRole.USER, content: expectedBlankSurface}, + {role: MessageRole.USER, content: expectedCompactBlankSurface, isSnapshot: true}, ]); // Simulate ChatCoordinator clearing history and calling resetToBlankDraft a second time in a row @@ -974,7 +1082,7 @@ describe('StateSync Autosave Draft Integrations', () => { service.resetToBlankDraft(); expect(service.sessionResetNonce()).toBe(2); expect(chatStateMock.chatHistory()).toEqual([ - {role: MessageRole.USER, content: expectedBlankSurface}, + {role: MessageRole.USER, content: expectedCompactBlankSurface, isSnapshot: true}, ]); // Simulate LLM committing layout, then clearing history and calling resetToBlankDraft again @@ -984,7 +1092,7 @@ describe('StateSync Autosave Draft Integrations', () => { expect(service.sessionResetNonce()).toBe(3); expect(service.activeDraft()).toBe(expectedBlankSurface); expect(chatStateMock.chatHistory()).toEqual([ - {role: MessageRole.USER, content: expectedBlankSurface}, + {role: MessageRole.USER, content: expectedCompactBlankSurface, isSnapshot: true}, ]); }); diff --git a/shell/src/app/chat/state-sync/state-sync.ts b/shell/src/app/chat/state-sync/state-sync.ts index 7c0ac8899..daa227b73 100644 --- a/shell/src/app/chat/state-sync/state-sync.ts +++ b/shell/src/app/chat/state-sync/state-sync.ts @@ -19,6 +19,7 @@ import {takeUntilDestroyed, toObservable} from '@angular/core/rxjs-interop'; import {of, Subject, timer} from 'rxjs'; import {filter, map, switchMap, takeUntil} from 'rxjs/operators'; import {ChatState} from '../chat-state/chat-state'; +import {ChatCleaner} from '../chat-cleaner/chat-cleaner'; import {MessageRole} from '../llm-client/llm-client'; import {CAR_BOOKING} from '../chat-service/initial-draft'; import {CatalogManagement} from '../../storage/catalog-management/catalog-management'; @@ -51,6 +52,7 @@ const MOCK_RULES_CONTAINER = 'mock_rules_container'; export class StateSync { private readonly destroyRef = inject(DestroyRef); private readonly chatState = inject(ChatState); + private readonly chatCleaner = inject(ChatCleaner); private readonly catalogManagement = inject(CatalogManagement); private readonly startupConfigState = inject(StartupConfigStateService); private readonly logger = inject(ErrorLogger).withTag('[StateSync]'); @@ -183,8 +185,10 @@ export class StateSync { commitLayoutFromLlm(value: string): void { this.cancelPendingDraftSync$.next(); this.isDraftEdited = true; - // Restoring a previous draft after this response must update context again. - this.lastSynchronizedLayout = null; + // Record the sanitized layout committed by the model so follow-up prompts without + // editor modifications do not append a duplicate USER snapshot, while restoring a + // previous draft still updates context. + this.lastSynchronizedLayout = this.sanitizeLayout(value) || null; this._activeDraft.set(value); } @@ -284,18 +288,16 @@ export class StateSync { { role: MessageRole.USER, content: sanitizedLayoutString, + isSnapshot: true, }, ]); return; } const lastMessage = history[history.length - 1]; - // Verify that the last message is indeed a user message AND starts with - // A2UI JSON array brackets. - // This ensures plain user text prompt dispatches are preserved - // completely intact! - const isLastMessageLayoutSnapshot = - lastMessage.role === MessageRole.USER && lastMessage.content.trim().startsWith('['); + // Verify that the last message is an unprompted user layout snapshot so + // plain user text prompts (including those starting with '[') are preserved intact. + const isLastMessageLayoutSnapshot = this.chatCleaner.isUserLayoutSnapshot(lastMessage); if (isLastMessageLayoutSnapshot) { // Overwrite trailing turn in-place to avoid array inflation @@ -304,6 +306,7 @@ export class StateSync { updatedHistory[updatedHistory.length - 1] = { role: MessageRole.USER, content: sanitizedLayoutString, + isSnapshot: true, }; this.chatState.setChatHistory(updatedHistory); } else { @@ -314,6 +317,7 @@ export class StateSync { { role: MessageRole.USER, content: sanitizedLayoutString, + isSnapshot: true, }, ]); } @@ -338,7 +342,7 @@ export class StateSync { return block; }) .filter((b): b is RenderA2uiItem => b !== null); - return formatJson(sanitized); + return sanitized.length > 0 ? JSON.stringify(sanitized) : ''; } this.logger.warn(