From 8b86e03d70cef3acc087dc338fd0ae47c430a19f Mon Sep 17 00:00:00 2001 From: Jay Gindin Date: Fri, 16 Oct 2026 12:10:21 -0400 Subject: [PATCH 1/6] Omit multimodal prompt guidelines on text-only turns Add memoized text-only and multimodal system prompt signals behind buildSystemPrompt({includeMultimodalGuidelines}) in ChatPromptFactoryService. Include multimodal guidelines in ChatCoordinator and ChatPanelBase only when active context or staged inputs contain an image or screenshot, and infer missing file MIME types in FileIngestionService via a2a-attachments helpers. --- .../chat-coordinator/chat-coordinator.spec.ts | 74 ++++++++ .../chat/chat-coordinator/chat-coordinator.ts | 37 ++-- .../app/chat/chat-panel/chat-panel-base.ts | 13 +- .../app/chat/chat-panel/chat-panel.spec.ts | 36 ++++ .../chat-prompt-factory.service.spec.ts | 36 ++++ .../chat-prompt-factory.service.ts | 168 ++++++++++-------- .../copilotkit-chat-panel.spec.ts | 3 + .../file-ingestion.service.spec.ts | 10 ++ .../file-ingestion/file-ingestion.service.ts | 6 +- 9 files changed, 296 insertions(+), 87 deletions(-) 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..5dad06051 100644 --- a/shell/src/app/chat/chat-coordinator/chat-coordinator.spec.ts +++ b/shell/src/app/chat/chat-coordinator/chat-coordinator.spec.ts @@ -258,6 +258,80 @@ describe('ChatCoordinator Pipeline & State Integration', () => { 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', () => { const flight = [ {version: 'v0.9', createSurface: {surfaceId: 'showcase-01-flight', catalogId: 'basic'}}, diff --git a/shell/src/app/chat/chat-coordinator/chat-coordinator.ts b/shell/src/app/chat/chat-coordinator/chat-coordinator.ts index c9f4e2cdf..101488657 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, @@ -99,7 +100,16 @@ export class ChatCoordinator { * 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.chatState + .chatHistory() + .some( + m => m.role !== MessageRole.ERROR && m.attachments?.some(a => this.isImageAttachment(a)), + ); + return this.promptFactory.buildSystemPrompt({ + includeMultimodalGuidelines: hasImageAttachment, + }); + }); private activePromptId: string | null = null; @@ -167,16 +177,27 @@ export class ChatCoordinator { this.chatState.setProgrammaticStreamActive(false); } + private isImageAttachment(attachment: Attachment): boolean { + return isA2aImageAttachment(attachment.mimeType, attachment.name); + } + /** * Constructs System instructions prepended message logs context. */ getFullMessageContext(): LlmMessage[] { + const nonErrorMessages = this.chatState.chatHistory().filter(m => m.role !== MessageRole.ERROR); + const hasImageAttachment = nonErrorMessages.some(m => + m.attachments?.some(a => this.isImageAttachment(a)), + ); + return [ { role: MessageRole.SYSTEM, - content: this.promptFactory.systemPrompt(), + content: this.promptFactory.buildSystemPrompt({ + includeMultimodalGuidelines: hasImageAttachment, + }), }, - ...this.chatState.chatHistory().filter(m => m.role !== MessageRole.ERROR), + ...nonErrorMessages, ]; } @@ -225,12 +246,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({ 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..94e34237f 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( @@ -356,7 +365,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..303334960 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, @@ -699,6 +702,39 @@ 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 and staging an image attachment also switches systemPrompt() to multimodal + await harness.toggleScreenshot(); + 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..0e24d0914 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 @@ -219,4 +219,40 @@ 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'); + }); + }); }); 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..e8a343c69 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 @@ -64,7 +64,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 +103,12 @@ export class ChatPromptFactoryService { } return ( - (this.generateSystemPrompt(formatJson(catalog)) + mcpInstructions).trimEnd() + - customPromptSuffix + ( + this.generateSystemPrompt(formatJson(catalog), includeMultimodalGuidelines) + + mcpInstructions + ).trimEnd() + customPromptSuffix ); - }); + } /** * Updates the custom instruction state and persists it to local storage. @@ -215,78 +231,16 @@ 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 - - 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(). + 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 +369,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 + ${formatJson(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 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..37be632da 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,14 @@ describe('FileIngestionService', () => { expect(result.data).toBe(btoa('blobdata')); expect(result.previewUrl).toContain('data:image/png;base64,'); }); + + it('infers image MIME type and previewUrl from filename extension when File.type is empty', async () => { + const file = new File(['webpdata'], 'wireframe.webp', {type: ''}); + const result = await service.readFileAsAttachment(file); + + expect(result.name).toBe('wireframe.webp'); + expect(result.mimeType).toBe('image/webp'); + expect(result.data).toBe(btoa('webpdata')); + expect(result.previewUrl).toBeDefined(); + }); }); 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..dbd6e86a9 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,12 @@ 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) ? result : undefined, }); }; reader.onerror = err => reject(err); From 4f0abe8e2a0bd843f12701a204c1dab9ce610bfb Mon Sep 17 00:00:00 2001 From: Jay Gindin Date: Fri, 16 Oct 2026 12:16:33 -0400 Subject: [PATCH 2/6] Minify injected JSON schemas in system prompt Serialize COMMON_TYPES_SCHEMA and the active catalog with compact JSON.stringify instead of pretty-printed formatJson. Prune unreferenced $defs entries from the catalog before prompt injection while preserving transitively referenced definitions and without mutating input objects. --- .../chat-coordinator/chat-coordinator.spec.ts | 2 +- .../chat-prompt-factory.service.spec.ts | 75 +++++++++++++++++++ .../chat-prompt-factory.service.ts | 63 +++++++++++++++- 3 files changed, 135 insertions(+), 5 deletions(-) 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 5dad06051..94c84a52a 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,7 @@ 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', () => { 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 0e24d0914..e3149b186 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'; @@ -255,4 +256,78 @@ describe('ChatPromptFactoryService', () => { 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'}); + }); + }); }); 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 e8a343c69..00b908f24 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. */ @@ -104,8 +106,10 @@ export class ChatPromptFactoryService { return ( ( - this.generateSystemPrompt(formatJson(catalog), includeMultimodalGuidelines) + - mcpInstructions + this.generateSystemPrompt( + this.serializeCatalogForPrompt(catalog), + includeMultimodalGuidelines, + ) + mcpInstructions ).trimEnd() + customPromptSuffix ); } @@ -231,6 +235,57 @@ export class ChatPromptFactoryService { `; } + /** + * 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); + } + + 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); + } + } + } + } + private generateSystemPrompt(catalog: string, includeMultimodalGuidelines = false): string { const multimodalGuidelinesSection = includeMultimodalGuidelines ? ` @@ -406,7 +461,7 @@ export class ChatPromptFactoryService { 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)} + ${MINIFIED_COMMON_TYPES_SCHEMA} \`\`\` ## Editing the Current UI From 34a1ce2b2306ec2c2bc45435b3d1990732c06709 Mon Sep 17 00:00:00 2001 From: Jay Gindin Date: Fri, 16 Oct 2026 12:20:19 -0400 Subject: [PATCH 3/6] Condense few-shot examples in system prompt Replace the Simple Example and verbose vacation_booking Complex Form Example in ChatPromptFactoryService with a compact form and action example while retaining the dynamic list template example. Document optional updateDataModel.path updates and clarify root vs. relative template paths in Data Binding. --- .../chat-prompt-factory.service.spec.ts | 82 +++++++++++++++++++ .../chat-prompt-factory.service.ts | 45 ++++------ 2 files changed, 100 insertions(+), 27 deletions(-) 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 e3149b186..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 @@ -330,4 +330,86 @@ describe('ChatPromptFactoryService', () => { 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 00b908f24..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 @@ -507,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 @@ -517,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. + **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. - * **Simple Example**: A basic column with text: + * **Form & Action Example**: \`\`\`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!"}} + {"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"}}} \`\`\` - * **Complex Form Example**: A vacation booking form demonstrating advanced - Material form controls (\`MaterialDatepicker\`, \`MaterialSelect\`, - \`MaterialSlideToggle\`) and buttons using the modernized Material catalog: + * **Dynamic List 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"}}} - \`\`\` - - * **Dynamic List Example**: An example using templates to render a list of - items. - \`\`\`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 From 2620aaca6a6ccf6406c569a571918fb43716dadd Mon Sep 17 00:00:00 2001 From: Jay Gindin Date: Fri, 16 Oct 2026 12:24:32 -0400 Subject: [PATCH 4/6] Serialize canvas layout snapshots as compact JSON in chat history Keep activeDraft() and Monaco editor text formatted via formatJson() while compacting sanitized layout snapshots and model responses written to chatHistory via JSON.stringify(). Add ChatCleaner isUserLayoutSnapshot(), stamp isSnapshot on synced layout messages, track LLM-committed layouts in lastSynchronizedLayout, and compute componentCount for user snapshots in ChatPanelBase. --- .../chat/chat-cleaner/chat-cleaner.spec.ts | 68 +++++++ .../src/app/chat/chat-cleaner/chat-cleaner.ts | 16 ++ .../chat-coordinator/chat-coordinator.spec.ts | 11 +- .../chat/chat-coordinator/chat-coordinator.ts | 9 +- .../app/chat/chat-panel/chat-panel-base.ts | 13 +- .../app/chat/chat-panel/chat-panel.spec.ts | 7 +- .../app/chat/state-sync/state-sync.spec.ts | 166 +++++++++++++++--- shell/src/app/chat/state-sync/state-sync.ts | 22 ++- 8 files changed, 257 insertions(+), 55 deletions(-) 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 94c84a52a..7397c2ba3 100644 --- a/shell/src/app/chat/chat-coordinator/chat-coordinator.spec.ts +++ b/shell/src/app/chat/chat-coordinator/chat-coordinator.spec.ts @@ -451,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 @@ -969,9 +972,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"'); diff --git a/shell/src/app/chat/chat-coordinator/chat-coordinator.ts b/shell/src/app/chat/chat-coordinator/chat-coordinator.ts index 101488657..35ead8770 100644 --- a/shell/src/app/chat/chat-coordinator/chat-coordinator.ts +++ b/shell/src/app/chat/chat-coordinator/chat-coordinator.ts @@ -504,18 +504,19 @@ 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 + // Update latest MODEL turn message in history to compact 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, + content: compactLayoutText, isSnapshot: true, componentCount, }; 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 94e34237f..8688819e0 100644 --- a/shell/src/app/chat/chat-panel/chat-panel-base.ts +++ b/shell/src/app/chat/chat-panel/chat-panel-base.ts @@ -194,7 +194,7 @@ export abstract class ChatPanelBase { componentCount: null, }; } - if (m.isSnapshot !== undefined) { + if (m.isSnapshot === false || (m.isSnapshot === true && m.componentCount != null)) { return { ...m, isSnapshot: m.isSnapshot, @@ -204,8 +204,8 @@ export abstract class ChatPanelBase { } const cleaned = m.content ? this.chatCleaner.cleanPayload(m.content) : ''; const parseResult = cleaned ? parseAndHealJsonLines(cleaned) : null; - let isSnapshot = false; - if (m.content && parseResult && parseResult.success) { + let isSnapshot = Boolean(m.isSnapshot); + if (!isSnapshot && m.content && parseResult && parseResult.success) { isSnapshot = !parseResult.isConversational; } if (isSnapshot && m.content) { @@ -336,9 +336,10 @@ 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 ?? + (Boolean(message.content) && this.chatCleaner.isLayoutSnapshot(message.content)); + return isLayoutSnapshot ? 'bubble-user bubble-layout' : 'bubble-user bubble-text'; } if (message.role === MessageRole.MODEL) { return 'bubble-model'; 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 303334960..077996d2b 100644 --- a/shell/src/app/chat/chat-panel/chat-panel.spec.ts +++ b/shell/src/app/chat/chat-panel/chat-panel.spec.ts @@ -224,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, @@ -247,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'); 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( From 203103c718bab56e15ec286b9c03f8f92bf32019 Mon Sep 17 00:00:00 2001 From: Jay Gindin Date: Fri, 16 Oct 2026 12:49:04 -0400 Subject: [PATCH 5/6] Exclude failed user prompt turns from LLM context on retry Filter out USER prompt messages and attachments paired with failed (ERROR or parseError) or cancelled (isCancelled) MODEL outcomes in ChatCoordinator.getFullMessageContext() and systemPrompt while keeping them in chatHistory for UI rendering. Preserve earlier succeeded turns when retries reuse promptId, deduplicate adjacent layout snapshots, and correlate MODEL placeholders by promptId. --- .../chat-coordinator/chat-coordinator.spec.ts | 243 ++++++++++++++++++ .../chat/chat-coordinator/chat-coordinator.ts | 198 ++++++++------ shell/src/app/chat/llm-client/llm-client.ts | 4 + 3 files changed, 370 insertions(+), 75 deletions(-) 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 7397c2ba3..8e699b32e 100644 --- a/shell/src/app/chat/chat-coordinator/chat-coordinator.spec.ts +++ b/shell/src/app/chat/chat-coordinator/chat-coordinator.spec.ts @@ -782,6 +782,8 @@ describe('ChatCoordinator Pipeline & State Integration', () => { { role: MessageRole.MODEL, content: '*This response was stopped because the renderer changed.*', + isCancelled: true, + promptId: 'prompt-1', }, ]); expect(service.pipelineStatus()).toBe(PipelineStatus.IDLE); @@ -1393,4 +1395,245 @@ 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', + ]); + }); + }); }); diff --git a/shell/src/app/chat/chat-coordinator/chat-coordinator.ts b/shell/src/app/chat/chat-coordinator/chat-coordinator.ts index 35ead8770..ec84b26ba 100644 --- a/shell/src/app/chat/chat-coordinator/chat-coordinator.ts +++ b/shell/src/app/chat/chat-coordinator/chat-coordinator.ts @@ -32,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'; @@ -56,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.*'; @@ -96,16 +94,18 @@ 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 = computed(() => { - const hasImageAttachment = this.chatState - .chatHistory() - .some( - m => m.role !== MessageRole.ERROR && m.attachments?.some(a => this.isImageAttachment(a)), - ); + const hasImageAttachment = this.conversationalMessages().some(m => + m.attachments?.some(a => this.isImageAttachment(a)), + ); return this.promptFactory.buildSystemPrompt({ includeMultimodalGuidelines: hasImageAttachment, }); @@ -181,23 +181,103 @@ export class ChatCoordinator { 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; + } + /** - * Constructs System instructions prepended message logs context. + * 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. */ - getFullMessageContext(): LlmMessage[] { - const nonErrorMessages = this.chatState.chatHistory().filter(m => m.role !== MessageRole.ERROR); - const hasImageAttachment = nonErrorMessages.some(m => - m.attachments?.some(a => this.isImageAttachment(a)), + 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 ? 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. + */ + getFullMessageContext(): LlmMessage[] { return [ { role: MessageRole.SYSTEM, - content: this.promptFactory.buildSystemPrompt({ - includeMultimodalGuidelines: hasImageAttachment, - }), + content: this.systemPrompt(), }, - ...nonErrorMessages, + ...this.conversationalMessages(), ]; } @@ -318,6 +398,7 @@ export class ChatCoordinator { { role: MessageRole.MODEL, content: this.chatCleaner.appendPulse(''), + promptId, }, ]); @@ -353,17 +434,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, }); } @@ -373,17 +448,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); @@ -395,19 +464,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); @@ -449,14 +512,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; } @@ -510,18 +566,10 @@ export class ChatCoordinator { const finalLayoutText = formatJson(parsedBlocks); // Update latest MODEL turn message in history to compact 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: compactLayoutText, - isSnapshot: true, - componentCount, - }; - } - return updated; + this.updateTargetModelMessage(promptId, { + content: compactLayoutText, + isSnapshot: true, + componentCount, }); // Commit layout synchronously to editor viewport before releasing @@ -654,7 +702,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, @@ -665,8 +713,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/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. From 59d3a0f5c9439e07f9bebe1ed2c4cf0d9412c7dd Mon Sep 17 00:00:00 2001 From: Jay Gindin Date: Mon, 19 Oct 2026 16:05:52 -0400 Subject: [PATCH 6/6] Refine snapshot detection, preview URLs, and error turn targeting Stamp isSnapshot: false on submitted USER prompt messages in ChatCoordinator.submitPrompt and gate USER snapshot inference on ChatCleaner.isUserLayoutSnapshot in ChatPanelBase.visibleChatHistory and getBubbleClass. Construct FileIngestionService previewUrl from the inferred MIME type and base64 payload, restrict the findTargetModelMessageIndex fallback to untagged MODEL turns, and expand unit test coverage. --- .../chat-coordinator/chat-coordinator.spec.ts | 146 +++++++++++++++++- .../chat/chat-coordinator/chat-coordinator.ts | 7 +- .../app/chat/chat-panel/chat-panel-base.ts | 19 +-- .../app/chat/chat-panel/chat-panel.spec.ts | 70 +++++++-- .../file-ingestion.service.spec.ts | 22 ++- .../file-ingestion/file-ingestion.service.ts | 4 +- 6 files changed, 238 insertions(+), 30 deletions(-) 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 8e699b32e..3a49aa343 100644 --- a/shell/src/app/chat/chat-coordinator/chat-coordinator.spec.ts +++ b/shell/src/app/chat/chat-coordinator/chat-coordinator.spec.ts @@ -778,7 +778,7 @@ 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.*', @@ -1635,5 +1635,149 @@ I'll output bad JSON. '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 ec84b26ba..718446b22 100644 --- a/shell/src/app/chat/chat-coordinator/chat-coordinator.ts +++ b/shell/src/app/chat/chat-coordinator/chat-coordinator.ts @@ -251,7 +251,11 @@ export class ChatCoordinator { } } const lastIdx = history.length - 1; - return lastIdx >= 0 && history[lastIdx].role === MessageRole.MODEL ? lastIdx : -1; + return lastIdx >= 0 && + history[lastIdx].role === MessageRole.MODEL && + (!promptId || !history[lastIdx].promptId) + ? lastIdx + : -1; } private updateTargetModelMessage(promptId: string | undefined, patch: Partial): void { @@ -383,6 +387,7 @@ export class ChatCoordinator { role: MessageRole.USER, content: trimmed, attachments: attachments.length > 0 ? attachments : undefined, + isSnapshot: false, promptId, }, ]); 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 8688819e0..765c35c71 100644 --- a/shell/src/app/chat/chat-panel/chat-panel-base.ts +++ b/shell/src/app/chat/chat-panel/chat-panel-base.ts @@ -194,20 +194,23 @@ export abstract class ChatPanelBase { componentCount: null, }; } - if (m.isSnapshot === false || (m.isSnapshot === true && m.componentCount != null)) { + 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 = Boolean(m.isSnapshot); - if (!isSnapshot && m.content && parseResult && parseResult.success) { - isSnapshot = !parseResult.isConversational; - } + const isSnapshot = + Boolean(m.isSnapshot) || Boolean(parseResult?.success && !parseResult.isConversational); if (isSnapshot && m.content) { return { ...m, @@ -336,9 +339,7 @@ export abstract class ChatPanelBase { */ protected getBubbleClass(message: LlmMessage): string { if (message.role === MessageRole.USER) { - const isLayoutSnapshot = - message.isSnapshot ?? - (Boolean(message.content) && this.chatCleaner.isLayoutSnapshot(message.content)); + const isLayoutSnapshot = message.isSnapshot ?? this.chatCleaner.isUserLayoutSnapshot(message); return isLayoutSnapshot ? 'bubble-user bubble-layout' : 'bubble-user bubble-text'; } if (message.role === MessageRole.MODEL) { 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 077996d2b..52a2e166d 100644 --- a/shell/src/app/chat/chat-panel/chat-panel.spec.ts +++ b/shell/src/app/chat/chat-panel/chat-panel.spec.ts @@ -260,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```', }, @@ -276,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```', }, @@ -296,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[] = [ { @@ -456,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"}]}} ●●●`, }, ]; @@ -468,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"}]}}', }, @@ -488,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'); }); @@ -715,8 +748,23 @@ describe('ChatPanel Gemini Dialogue Panel Integration', () => { includeMultimodalGuidelines: true, }); - // Disabling includeScreenshot and staging an image attachment also switches systemPrompt() to multimodal + // 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([ { 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 37be632da..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 @@ -46,13 +46,21 @@ describe('FileIngestionService', () => { expect(result.previewUrl).toContain('data:image/png;base64,'); }); - it('infers image MIME type and previewUrl from filename extension when File.type is empty', async () => { - const file = new File(['webpdata'], 'wireframe.webp', {type: ''}); - const result = await service.readFileAsAttachment(file); + 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(result.name).toBe('wireframe.webp'); - expect(result.mimeType).toBe('image/webp'); - expect(result.data).toBe(btoa('webpdata')); - expect(result.previewUrl).toBeDefined(); + 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 dbd6e86a9..806627e13 100644 --- a/shell/src/app/chat/file-ingestion/file-ingestion.service.ts +++ b/shell/src/app/chat/file-ingestion/file-ingestion.service.ts @@ -57,7 +57,9 @@ export class FileIngestionService { name: file.name, mimeType, data: base64Data, - previewUrl: isImageAttachment(mimeType, file.name) ? result : undefined, + previewUrl: isImageAttachment(mimeType, file.name) + ? `data:${mimeType};base64,${base64Data}` + : undefined, }); }; reader.onerror = err => reject(err);