-
Notifications
You must be signed in to change notification settings - Fork 415
fix(ai-agents): artifact streaming shows only first chunk after React Compiler #2280
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,33 +9,49 @@ | |||||||||||||
| * by the Apache License, Version 2.0 | ||||||||||||||
| */ | ||||||||||||||
|
|
||||||||||||||
| import type { TaskStatusUpdateEvent } from '@a2a-js/sdk'; | ||||||||||||||
| import type { TaskArtifactUpdateEvent, TaskStatusUpdateEvent } from '@a2a-js/sdk'; | ||||||||||||||
| import { describe, expect, vi } from 'vitest'; | ||||||||||||||
|
|
||||||||||||||
| import { handleStatusUpdateEvent } from './event-handlers'; | ||||||||||||||
| import { handleArtifactUpdateEvent, handleStatusUpdateEvent } from './event-handlers'; | ||||||||||||||
| import type { StreamingState } from './streaming-types'; | ||||||||||||||
| import type { ChatMessage } from '../types'; | ||||||||||||||
|
|
||||||||||||||
| describe('artifact duplication bug', () => { | ||||||||||||||
| const createMockState = (): StreamingState => ({ | ||||||||||||||
| contentBlocks: [], | ||||||||||||||
| activeTextBlock: null, | ||||||||||||||
| lastEventTimestamp: new Date(), | ||||||||||||||
| capturedTaskId: undefined, | ||||||||||||||
| capturedTaskState: undefined, | ||||||||||||||
| previousTaskState: undefined, | ||||||||||||||
| taskIdCapturedAtBlockIndex: undefined, | ||||||||||||||
| latestUsage: undefined, | ||||||||||||||
| }); | ||||||||||||||
| const createMockState = (overrides?: Partial<StreamingState>): StreamingState => ({ | ||||||||||||||
| contentBlocks: [], | ||||||||||||||
| activeTextBlock: null, | ||||||||||||||
| lastEventTimestamp: new Date(), | ||||||||||||||
| capturedTaskId: undefined, | ||||||||||||||
| capturedTaskState: undefined, | ||||||||||||||
| previousTaskState: undefined, | ||||||||||||||
| taskIdCapturedAtBlockIndex: undefined, | ||||||||||||||
| latestUsage: undefined, | ||||||||||||||
| ...overrides, | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| const createMockMessage = (): ChatMessage => ({ | ||||||||||||||
| id: 'test-msg-1', | ||||||||||||||
| role: 'assistant', | ||||||||||||||
| contextId: 'test-context', | ||||||||||||||
| timestamp: new Date(), | ||||||||||||||
| contentBlocks: [], | ||||||||||||||
| }); | ||||||||||||||
| const createMockMessage = (): ChatMessage => ({ | ||||||||||||||
| id: 'test-msg-1', | ||||||||||||||
| role: 'assistant', | ||||||||||||||
| contextId: 'test-context', | ||||||||||||||
| timestamp: new Date(), | ||||||||||||||
| contentBlocks: [], | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| const makeArtifactEvent = ( | ||||||||||||||
| text: string, | ||||||||||||||
| opts: { append?: boolean; lastChunk?: boolean } = {}, | ||||||||||||||
| ): TaskArtifactUpdateEvent => ({ | ||||||||||||||
| kind: 'artifact-update', | ||||||||||||||
| contextId: 'test-context', | ||||||||||||||
| taskId: 'task-123', | ||||||||||||||
| append: opts.append, | ||||||||||||||
| lastChunk: opts.lastChunk, | ||||||||||||||
| artifact: { | ||||||||||||||
| artifactId: 'artifact-1', | ||||||||||||||
| parts: text ? [{ kind: 'text', text }] : [], | ||||||||||||||
| }, | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| describe('handleStatusUpdateEvent', () => { | ||||||||||||||
| test('should allow normal agent messages through', () => { | ||||||||||||||
| const state = createMockState(); | ||||||||||||||
| const assistantMessage = createMockMessage(); | ||||||||||||||
|
|
@@ -56,7 +72,7 @@ describe('artifact duplication bug', () => { | |||||||||||||
| parts: [ | ||||||||||||||
| { | ||||||||||||||
| kind: 'text', | ||||||||||||||
| text: 'Artifact created successfully.\n\n- Name: Test Markdown Artifact\n- Description: A concise markdown sample\n- Artifact ID: artifact-d3trg0tuui6c73cprmj0', | ||||||||||||||
| text: 'Artifact created successfully.\n\n- Name: Test Markdown Artifact', | ||||||||||||||
| }, | ||||||||||||||
| ], | ||||||||||||||
| }, | ||||||||||||||
|
|
@@ -65,9 +81,83 @@ describe('artifact duplication bug', () => { | |||||||||||||
|
|
||||||||||||||
| handleStatusUpdateEvent(event, state, assistantMessage, onMessageUpdate); | ||||||||||||||
|
|
||||||||||||||
| // SHOULD create task-status-update block (normal message, not tool-related) | ||||||||||||||
| const statusBlocks = state.contentBlocks.filter((b) => b.type === 'task-status-update'); | ||||||||||||||
| expect(statusBlocks).toHaveLength(1); | ||||||||||||||
| expect(statusBlocks[0].text).toContain('Artifact created successfully'); | ||||||||||||||
| expect(statusBlocks[0].type === 'task-status-update' && statusBlocks[0].text).toContain( | ||||||||||||||
| 'Artifact created successfully', | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Biome] reported by reviewdog 🐶
Suggested change
|
||||||||||||||
| ); | ||||||||||||||
| }); | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| describe('handleArtifactUpdateEvent', () => { | ||||||||||||||
| /** | ||||||||||||||
| * Regression test for React Compiler memoization bug. | ||||||||||||||
| * | ||||||||||||||
| * The handler mutated activeTextBlock in place and passed the same reference | ||||||||||||||
| * to onMessageUpdate. React Compiler's auto-memoization detected the unchanged | ||||||||||||||
| * reference and skipped re-renders, so only the first chunk was displayed. | ||||||||||||||
| * | ||||||||||||||
| * The fix clones the artifact block for each UI update so React sees a new | ||||||||||||||
| * object on every call. | ||||||||||||||
| */ | ||||||||||||||
| test('each streaming chunk must produce a new artifact block reference', () => { | ||||||||||||||
| const state = createMockState({ capturedTaskId: 'task-123' }); | ||||||||||||||
| const assistantMessage = createMockMessage(); | ||||||||||||||
| const onMessageUpdate = vi.fn(); | ||||||||||||||
|
|
||||||||||||||
| // Simulate 3 streaming chunks | ||||||||||||||
| handleArtifactUpdateEvent(makeArtifactEvent('Hello'), state, assistantMessage, onMessageUpdate); | ||||||||||||||
| handleArtifactUpdateEvent(makeArtifactEvent(' world', { append: true }), state, assistantMessage, onMessageUpdate); | ||||||||||||||
| handleArtifactUpdateEvent(makeArtifactEvent('!', { append: true }), state, assistantMessage, onMessageUpdate); | ||||||||||||||
|
|
||||||||||||||
| expect(onMessageUpdate).toHaveBeenCalledTimes(3); | ||||||||||||||
|
|
||||||||||||||
| const getArtifactBlock = (callIndex: number) => { | ||||||||||||||
| const msg = onMessageUpdate.mock.calls[callIndex][0] as ChatMessage; | ||||||||||||||
| return msg.contentBlocks.find((b) => b.type === 'artifact'); | ||||||||||||||
| }; | ||||||||||||||
|
|
||||||||||||||
| const block1 = getArtifactBlock(0); | ||||||||||||||
| const block2 = getArtifactBlock(1); | ||||||||||||||
| const block3 = getArtifactBlock(2); | ||||||||||||||
|
|
||||||||||||||
| // References must differ so React detects the change | ||||||||||||||
| expect(block1).not.toBe(block2); | ||||||||||||||
| expect(block2).not.toBe(block3); | ||||||||||||||
|
|
||||||||||||||
| // Text must accumulate | ||||||||||||||
| expect(block1?.type === 'artifact' && block1.parts[0]?.kind === 'text' && block1.parts[0].text).toBe('Hello'); | ||||||||||||||
| expect(block2?.type === 'artifact' && block2.parts[0]?.kind === 'text' && block2.parts[0].text).toBe( | ||||||||||||||
| 'Hello world', | ||||||||||||||
| ); | ||||||||||||||
|
Comment on lines
+130
to
+132
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Biome] reported by reviewdog 🐶
Suggested change
|
||||||||||||||
| expect(block3?.type === 'artifact' && block3.parts[0]?.kind === 'text' && block3.parts[0].text).toBe( | ||||||||||||||
| 'Hello world!', | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Biome] reported by reviewdog 🐶
Suggested change
|
||||||||||||||
| ); | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| test('lastChunk finalizes the artifact into contentBlocks', () => { | ||||||||||||||
| const state = createMockState({ capturedTaskId: 'task-123' }); | ||||||||||||||
| const assistantMessage = createMockMessage(); | ||||||||||||||
| const onMessageUpdate = vi.fn(); | ||||||||||||||
|
|
||||||||||||||
| handleArtifactUpdateEvent(makeArtifactEvent('Hello'), state, assistantMessage, onMessageUpdate); | ||||||||||||||
| handleArtifactUpdateEvent(makeArtifactEvent(' world', { append: true }), state, assistantMessage, onMessageUpdate); | ||||||||||||||
| // lastChunk with empty parts (matches real backend behavior) | ||||||||||||||
| handleArtifactUpdateEvent( | ||||||||||||||
| makeArtifactEvent('', { append: true, lastChunk: true }), | ||||||||||||||
| state, | ||||||||||||||
| assistantMessage, | ||||||||||||||
| onMessageUpdate, | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Biome] reported by reviewdog 🐶
Suggested change
|
||||||||||||||
| ); | ||||||||||||||
|
|
||||||||||||||
| // activeTextBlock should be cleared | ||||||||||||||
| expect(state.activeTextBlock).toBeNull(); | ||||||||||||||
|
|
||||||||||||||
| // Artifact should be persisted in contentBlocks with accumulated text | ||||||||||||||
| const artifacts = state.contentBlocks.filter((b) => b.type === 'artifact'); | ||||||||||||||
| expect(artifacts).toHaveLength(1); | ||||||||||||||
| expect(artifacts[0].type === 'artifact' && artifacts[0].parts[0]?.kind === 'text' && artifacts[0].parts[0].text).toBe( | ||||||||||||||
| 'Hello world', | ||||||||||||||
| ); | ||||||||||||||
|
Comment on lines
+159
to
+161
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Biome] reported by reviewdog 🐶
Suggested change
|
||||||||||||||
| }); | ||||||||||||||
| }); | ||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Biome] reported by reviewdog 🐶