From 6809cd7353724da10b70ae18b319622f363e4088 Mon Sep 17 00:00:00 2001 From: miinhho Date: Wed, 21 Oct 2026 10:02:48 +0900 Subject: [PATCH 01/10] [ZEPPELIN-6748] Prepare assistant components for conversation panel assembly --- .../assistant/ui/AssistantAnswer.spec.tsx | 83 ++++++++++++++++- .../entities/assistant/ui/AssistantAnswer.tsx | 91 ++++++++++++------- .../assistant/ui/AssistantComposer.tsx | 8 +- .../assistant/ui/ConversationHeader.css.ts | 17 ++++ .../ui/ConversationHeader.stories.tsx | 2 +- .../assistant/ui/ConversationHeader.tsx | 36 +++++++- .../assistant/ui/ConversationList.tsx | 12 ++- 7 files changed, 208 insertions(+), 41 deletions(-) diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantAnswer.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantAnswer.spec.tsx index 50edf035857..17afd101dc1 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantAnswer.spec.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantAnswer.spec.tsx @@ -20,6 +20,7 @@ const answer = () => screen.getByRole('article', { name: 'Assistant answer' }); describe('AssistantAnswer', () => { it('shows only the current state as the run moves from loading to done', () => { const { rerender } = render(); + expect(screen.getByRole('status').textContent).toBe('Thinking…'); expect(answer().getAttribute('aria-busy')).toBe('true'); @@ -32,6 +33,7 @@ describe('AssistantAnswer', () => { }} /> ); + expect(screen.getByRole('status').textContent).toBe('Writing answer…'); expect(answer().textContent).toContain('Revenue is grouped'); @@ -50,6 +52,7 @@ describe('AssistantAnswer', () => { }} /> ); + expect(screen.queryByRole('status')).toBeNull(); expect(screen.queryByRole('alert')).toBeNull(); expect(answer().hasAttribute('aria-busy')).toBe(false); @@ -70,6 +73,7 @@ describe('AssistantAnswer', () => { }} /> ); + expect(screen.getByRole('status').textContent).toBe('Running read_results…'); expect(screen.getByText('Using 2 tools')).toBeTruthy(); const [first, second] = screen.getAllByRole('listitem'); @@ -77,19 +81,22 @@ describe('AssistantAnswer', () => { expect(within(second).getByRole('img', { name: 'Running' })).toBeTruthy(); }); - it('opens the tool log while a tool runs and collapses it when the answer completes', () => { + it('keeps a user-collapsed tool log closed through answer updates and completion', () => { const running: AssistantAnswerView = { status: 'streaming', content: '', tools: [{ id: 't1', name: 'list_paragraphs', status: 'running' }] }; - const toolLog = () => screen.getByText(/tool$/).closest('details')!; + const toolLog = () => within(answer()).getByRole('group') as HTMLDetailsElement; + const { rerender } = render(); + expect(toolLog().open).toBe(true); // A user who collapses the log keeps it collapsed while more of the answer arrives. toolLog().open = false; rerender(); + expect(toolLog().open).toBe(false); rerender( @@ -104,11 +111,80 @@ describe('AssistantAnswer', () => { }} /> ); + expect(toolLog().open).toBe(false); }); + it('preserves an explicitly expanded finished tool log when streaming completes', () => { + const { rerender } = render( + + ); + + const log = within(answer()).getByRole('group') as HTMLDetailsElement; + const summary = log.querySelector('summary')!; + log.open = true; + summary.focus(); + + rerender( + + ); + + expect(within(answer()).getByRole('group')).toHaveProperty('open', true); + expect(document.activeElement).toBe(summary); + }); + + it('keeps the native tool log and its focused summary when streaming fails', () => { + const tools = [ + { id: 't1', name: 'list_paragraphs', status: 'done' as const }, + { id: 't2', name: 'read_results', status: 'running' as const } + ]; + const { rerender } = render(); + const log = within(answer()).getByRole('group') as HTMLDetailsElement; + const summary = log.querySelector('summary')!; + log.open = false; + summary.focus(); + + rerender( + + ); + + expect(within(answer()).getByRole('group')).toBe(log); + expect(log.open).toBe(false); + expect(document.activeElement).toBe(summary); + expect(screen.queryByRole('status')).toBeNull(); + expect(screen.getByRole('alert').textContent).toContain('Connection lost'); + expect(answer().textContent).toContain('Revenue'); + expect(screen.getByRole('img', { name: 'Done' })).toBeTruthy(); + expect(screen.getByRole('img', { name: 'Status unknown' })).toBeTruthy(); + expect(screen.queryByRole('img', { name: 'Running' })).toBeNull(); + }); + it('labels a tool whose completion event carried no name', () => { render(); + expect(screen.getByRole('status').textContent).toBe('Running Tool…'); }); @@ -141,7 +217,9 @@ describe('AssistantAnswer', () => { message: 'The assistant stopped responding before the reply finished. Retry the request.', content: 'Revenue is grouped' }; + render(); + expect(screen.queryByRole('status')).toBeNull(); expect(answer().hasAttribute('aria-busy')).toBe(false); expect(answer().textContent).toContain('Revenue is grouped'); @@ -175,6 +253,7 @@ describe('AssistantAnswer', () => { renderContent={content => {content.replaceAll('*', '')}} /> ); + expect(answer().querySelector('strong')?.textContent).toBe('bold'); }); }); diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantAnswer.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantAnswer.tsx index 1139c73b2eb..c159194c7f0 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantAnswer.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantAnswer.tsx @@ -11,6 +11,7 @@ */ import { Skeleton } from 'antd'; +import type { ReactNode } from 'react'; import { answerText, answerToolCalls } from '../model/messageHistory'; import type { AssistantContentRenderer, AssistantAnswerView, AssistantToolActivity } from './answerView'; import { AssistantAnswerError } from './AssistantAnswerError'; @@ -32,6 +33,7 @@ const Progress = ({ text }: { text: string }) => ( const progressText = (content: string, tools: AssistantToolActivity[]) => { const running = tools.find(tool => tool.status === 'running'); if (running) return `Running ${toolLabel(running)}…`; + return content ? 'Writing answer…' : 'Thinking…'; }; @@ -42,43 +44,66 @@ export interface AssistantAnswerProps { renderContent?: AssistantContentRenderer; } -/** One assistant answer in whichever state its run is in. Each state replaces the last, so only one shows at a time. */ -export const AssistantAnswer = ({ view, onRetry, renderContent }: AssistantAnswerProps) => { +const renderAnswerState = ( + view: AssistantAnswerView, + onRetry?: () => void +): { content: string; tools: AssistantToolActivity[]; streaming: boolean; details: ReactNode } => { switch (view.status) { case 'loading': - return ( - - - - - ); + return { + content: '', + tools: [], + streaming: true, + details: ( + <> + + + + ) + }; + case 'streaming': { const tools = view.tools ?? []; - return ( - - - - - ); - } - case 'done': { - const tools = answerToolCalls(view.message).map(call => ({ ...call, status: 'done' as const })); - return ( - - - - ); - } - case 'error': { - const tools = (view.tools ?? []).map(tool => - tool.status === 'running' ? { ...tool, status: 'unknown' as const } : tool - ); - return ( - - - - - ); + return { + content: view.content, + tools, + streaming: true, + details: + }; } + + case 'done': + return { + content: answerText(view.message), + tools: answerToolCalls(view.message).map(call => ({ ...call, status: 'done' as const })), + streaming: false, + details: null + }; + + case 'error': + return { + content: view.content ?? '', + tools: (view.tools ?? []).map(tool => + tool.status === 'running' ? { ...tool, status: 'unknown' as const } : tool + ), + streaming: false, + details: + }; } }; + +export const AssistantAnswer = ({ view, onRetry, renderContent }: AssistantAnswerProps) => { + const presentation = renderAnswerState(view, onRetry); + + return ( + + + {presentation.details} + + ); +}; diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantComposer.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantComposer.tsx index d0ede4c8099..c07d677914e 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantComposer.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantComposer.tsx @@ -81,7 +81,13 @@ export const AssistantComposer = ({ Shift + Enter for a new line - diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.css.ts b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.css.ts index c2a93b83978..389c0e0f419 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.css.ts +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.css.ts @@ -33,3 +33,20 @@ export const lock = style({ marginRight: vars.space1, color: vars.textTertiary }); + +export const listTrigger = style({ + display: 'inline-flex', + alignItems: 'center', + justifyContent: 'center', + flexShrink: 0, + width: 32, + height: 32, + border: 0, + borderRadius: vars.radiusSm, + background: 'transparent', + color: vars.text, + cursor: 'pointer', + selectors: { + '&:hover': { background: vars.fill } + } +}); diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.stories.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.stories.tsx index 403f0b32867..3aa96ea1ee3 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.stories.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.stories.tsx @@ -33,7 +33,7 @@ const readOnly: AssistantConversation = { const meta = { title: 'Assistant/ConversationHeader', component: ConversationHeader, - args: { conversation: current, onCreate: fn(), onDelete: fn() } + args: { conversation: current, onCreate: fn(), onDelete: fn(), onOpenList: fn() } } satisfies Meta; export default meta; diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.tsx index c7c1f8e3add..200af66f45a 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.tsx @@ -10,7 +10,8 @@ * limitations under the License. */ -import { DeleteOutlined, LockOutlined, PlusOutlined } from '@ant-design/icons'; +import type { Ref } from 'react'; +import { DeleteOutlined, LockOutlined, PlusOutlined, UnorderedListOutlined } from '@ant-design/icons'; import { Button } from 'antd'; import { visuallyHidden } from '@/shared/ui/assistant-theme/AssistantScope.css'; import type { AssistantConversation } from '../model/assistantContract'; @@ -20,14 +21,36 @@ import * as styles from './ConversationHeader.css'; export interface ConversationHeaderProps { conversation?: AssistantConversation; onCreate: () => void; + /** Disables only the New conversation action. */ + newDisabled?: boolean; onDelete: (conversationId: string) => void; + onOpenList?: () => void; + listTriggerRef?: Ref; } -export const ConversationHeader = ({ conversation, onCreate, onDelete }: ConversationHeaderProps) => { +export const ConversationHeader = ({ + conversation, + onCreate, + newDisabled, + onDelete, + onOpenList, + listTriggerRef +}: ConversationHeaderProps) => { const title = conversation ? conversationTitle(conversation) : 'No conversation selected'; const readOnly = conversation?.canSendMessage === false; return (
+ {onOpenList && ( + + )} {readOnly && ( <>
+
, panel.element @@ -114,7 +125,7 @@ export const mount = (element: HTMLElement, initialProps: AssistantWorkspaceProp const renderWith = (props: AssistantWorkspaceProps) => { root.render( - + diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/lib/useConversationScroll.ts b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/lib/useConversationScroll.ts new file mode 100644 index 00000000000..c1838273a53 --- /dev/null +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/lib/useConversationScroll.ts @@ -0,0 +1,296 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { useLayoutEffect, useRef } from 'react'; + +export type ConversationRowSegments = ReadonlyMap; + +interface Anchor { + row: HTMLElement; + block: HTMLElement; + text: string; + blockIndex: number; + blockCount: number; + firstSegment?: string; + blockOffset: number; + rowOffset: number; + neighbors: Array<{ row: HTMLElement; offset: number }>; +} + +interface ScrollState { + scope: object; + key: string; + follow: boolean; + anchor?: Anchor; + expectedTop?: number; + interacting: boolean; + top: number; + height: number; + width: number; + contentHeight: number; +} + +const markdownBlocks = + '[data-conversation-markdown] p, [data-conversation-markdown] pre, ' + + '[data-conversation-markdown] table, [data-conversation-markdown] li, ' + + '[data-conversation-markdown] h1, [data-conversation-markdown] h2, ' + + '[data-conversation-markdown] h3, [data-conversation-markdown] blockquote'; + +const captureAnchor = ( + viewport: HTMLElement, + content: HTMLElement, + rows: ConversationRowSegments +): Anchor | undefined => { + const top = viewport.getBoundingClientRect().top; + const bottom = top + viewport.clientHeight; + + for (const row of Array.from(content.querySelectorAll('[data-message-key]'))) { + const rect = row.getBoundingClientRect(); + if (rect.bottom <= top || rect.top >= bottom) continue; + + // Anchor within an answer because older pages can extend that same row. + const blocks = Array.from(row.querySelectorAll(markdownBlocks)); + const block = + blocks.find(element => { + const bounds = element.getBoundingClientRect(); + return bounds.bottom > top && bounds.top < bottom; + }) ?? row; + + const neighbors = [row.nextElementSibling, row.previousElementSibling] + .filter( + (element): element is HTMLElement => element instanceof HTMLElement && element.hasAttribute('data-message-key') + ) + .map(element => ({ row: element, offset: element.getBoundingClientRect().top - top })); + + return { + row, + block, + text: block.textContent ?? '', + blockIndex: blocks.indexOf(block), + blockCount: blocks.length, + firstSegment: rows.get(row.dataset.messageKey!)?.[0]?.id, + blockOffset: block.getBoundingClientRect().top - top, + rowOffset: rect.top - top, + neighbors + }; + } + + return undefined; +}; + +/** Keeps the reader's position when rendered messages or viewport dimensions change. */ +export const useConversationScroll = ({ + scope, + conversationKey, + rowSegments, + visible, + loadEarlier +}: { + scope: object; + conversationKey: string; + rowSegments: ConversationRowSegments; + visible: boolean; + loadEarlier: () => Promise; +}) => { + const viewport = useRef(null); + const content = useRef(null); + const state = useRef(undefined); + + const latest = useRef({ rowSegments, loadEarlier }); + latest.current = { rowSegments, loadEarlier }; + + const reconcile = useRef<() => void>(() => undefined); + const schedule = useRef<() => void>(() => undefined); + + useLayoutEffect(() => { + if (!state.current || state.current.scope !== scope || state.current.key !== conversationKey) { + state.current = { + scope, + key: conversationKey, + follow: true, + interacting: false, + top: 0, + height: 0, + width: 0, + contentHeight: 0 + }; + } + + const current = state.current; + const element = viewport.current; + const stack = content.current; + if (!visible || !element || !stack) return; + + let disposed = false; + let frame: number | undefined; + + const writeTop = (top: number) => { + const target = Math.max(0, Math.min(top, element.scrollHeight - element.clientHeight)); + if (Math.abs(element.scrollTop - target) <= 1) return; + + element.scrollTop = target; + current.expectedTop = element.scrollTop; + }; + + const update = () => { + if (disposed || element.clientHeight === 0 || element.clientWidth === 0) return; + + const geometryChanged = + current.height !== element.clientHeight || + current.width !== element.clientWidth || + current.contentHeight !== element.scrollHeight; + + if (current.follow) { + if (geometryChanged) writeTop(element.scrollHeight); + } else if (current.anchor) { + const { row, block, text, blockIndex, blockCount, firstSegment, rowOffset, blockOffset, neighbors } = + current.anchor; + const blocks = stack.contains(row) ? Array.from(row.querySelectorAll(markdownBlocks)) : []; + + // Repeated paragraphs need segment identity, not text matching alone. + const prepended = + firstSegment && + (latest.current.rowSegments.get(row.dataset.messageKey!)?.findIndex(segment => segment.id === firstSegment) ?? + -1) > 0; + const index = blockIndex + (prepended ? blocks.length - blockCount : 0); + const indexed = blocks[index]?.textContent?.startsWith(text) ? blocks[index] : undefined; + const retainedBlock = + prepended && indexed + ? indexed + : stack.contains(block) && block.textContent?.startsWith(text) + ? block + : (indexed ?? blocks.find(candidate => candidate.textContent === text)); + + const fallback = stack.contains(row) + ? { row, offset: rowOffset } + : neighbors.find(candidate => stack.contains(candidate.row)); + const target = retainedBlock ?? fallback?.row; + + if (target) { + const offset = retainedBlock ? blockOffset : fallback!.offset; + + // Exclude resize clamping so anchor correction preserves native scrolling. + const clampedTop = Math.max(0, Math.min(current.top, element.scrollHeight - element.clientHeight)); + const readerMovement = element.scrollTop - clampedTop; + writeTop( + element.scrollTop + + target.getBoundingClientRect().top - + element.getBoundingClientRect().top - + offset + + readerMovement + ); + } + } + + current.anchor = current.follow ? undefined : captureAnchor(element, stack, latest.current.rowSegments); + current.height = element.clientHeight; + current.width = element.clientWidth; + current.contentHeight = element.scrollHeight; + current.top = element.scrollTop; + + if (element.scrollHeight <= element.clientHeight + 1 || element.scrollTop <= 16) { + void latest.current.loadEarlier().then(loaded => { + if (loaded && !disposed) schedule.current(); + }); + } + }; + + const queue = () => { + if (disposed || frame !== undefined) return; + frame = requestAnimationFrame(() => { + frame = undefined; + update(); + }); + }; + + reconcile.current = update; + schedule.current = queue; + + const interaction = (event: Event) => { + if (event instanceof KeyboardEvent) { + if (!['ArrowUp', 'ArrowDown', 'PageUp', 'PageDown', 'Home', 'End', ' '].includes(event.key)) return; + if ( + event.target instanceof Element && + event.target.closest('input, textarea, select, button, summary, [contenteditable]') + ) + return; + } + if (event.type === 'pointerdown' && event.target !== element) return; + + current.interacting = true; + current.expectedTop = undefined; + }; + + element.addEventListener('wheel', interaction, { passive: true }); + element.addEventListener('touchmove', interaction, { passive: true }); + element.addEventListener('keydown', interaction); + element.addEventListener('pointerdown', interaction); + + const observer = new ResizeObserver(queue); + observer.observe(element); + observer.observe(stack); + update(); + + return () => { + disposed = true; + observer.disconnect(); + element.removeEventListener('wheel', interaction); + element.removeEventListener('touchmove', interaction); + element.removeEventListener('keydown', interaction); + element.removeEventListener('pointerdown', interaction); + if (frame !== undefined) cancelAnimationFrame(frame); + reconcile.current = () => undefined; + schedule.current = () => undefined; + }; + }, [scope, conversationKey, visible]); + + useLayoutEffect(() => { + if (visible) reconcile.current(); + }); + + const onScroll = () => { + const element = viewport.current; + const stack = content.current; + const current = state.current; + if (!visible || !element || !stack || !current || element.clientHeight === 0) return; + + if ( + !current.interacting && + current.expectedTop !== undefined && + Math.abs(element.scrollTop - current.expectedTop) <= 1 + ) { + current.expectedTop = undefined; + return; + } + + current.expectedTop = undefined; + // Resize-generated scroll events must not change follow mode. + if ( + !current.interacting && + (current.height !== element.clientHeight || + current.width !== element.clientWidth || + current.contentHeight !== element.scrollHeight) + ) { + schedule.current(); + return; + } + + current.interacting = false; + current.follow = + element.scrollTop >= current.top - 1 && element.scrollHeight - element.scrollTop - element.clientHeight <= 48; + current.top = element.scrollTop; + current.anchor = current.follow ? undefined : captureAnchor(element, stack, latest.current.rowSegments); + schedule.current(); + }; + + return { viewport, content, onScroll }; +}; diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/lib/useDeleteConfirmation.ts b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/lib/useDeleteConfirmation.ts new file mode 100644 index 00000000000..ea1719f155d --- /dev/null +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/lib/useDeleteConfirmation.ts @@ -0,0 +1,135 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { useCallback, useLayoutEffect, useRef, useState } from 'react'; +import type { AssistantConversation } from '@/entities/assistant/model/assistantContract'; +import { conversationTitle } from '@/entities/assistant/ui/conversationTitle'; + +interface DeleteConfirmationOptions { + scope: object; + conversations: readonly AssistantConversation[]; + listLoading: boolean; + deletingConversationIds: readonly string[]; + error?: string; + onDelete: (id: string) => Promise; + fallbackFocus: () => HTMLElement | null; +} + +interface DeleteTarget { + id: string; + title: string; +} + +type DeleteInteraction = { + scope: object; + trigger: HTMLElement | null; +} & ({ kind: 'open'; target: DeleteTarget; status: 'ready' | 'pending' | 'failed' } | { kind: 'closed' }); + +/** Closing the confirmation does not cancel an already requested deletion. */ +export const useDeleteConfirmation = ({ + scope, + conversations, + listLoading, + deletingConversationIds, + error, + onDelete, + fallbackFocus +}: DeleteConfirmationOptions) => { + const [interaction, setInteraction] = useState(null); + const current = useRef(null); + + // Event handlers must see state changes before React commits. + const transition = useCallback((next: DeleteInteraction | null) => { + current.current = next; + setInteraction(next); + }, []); + + useLayoutEffect(() => { + transition(null); + + return () => { + current.current = null; + }; + }, [scope, transition]); + + useLayoutEffect(() => { + if ( + interaction?.scope !== scope || + interaction.kind !== 'open' || + listLoading || + conversations.some(conversation => conversation.id === interaction.target.id) + ) + return; + + transition({ kind: 'closed', scope, trigger: interaction.trigger }); + }, [interaction, scope, listLoading, conversations, transition]); + + useLayoutEffect(() => { + if (interaction?.scope !== scope || interaction.kind !== 'closed') return; + + transition(null); + + const target = interaction.trigger?.isConnected ? interaction.trigger : fallbackFocus(); + target?.focus(); + }, [interaction, scope, fallbackFocus, transition]); + + const request = (id: string) => { + const conversation = conversations.find(item => item.id === id); + if (!conversation || conversation.canSendMessage === false) return; + + transition({ + kind: 'open', + scope, + target: { id, title: conversationTitle(conversation) }, + trigger: document.activeElement instanceof HTMLElement ? document.activeElement : null, + status: 'ready' + }); + }; + + const cancel = () => { + const active = current.current; + if (active?.scope !== scope || active.kind !== 'open') return; + + transition({ kind: 'closed', scope, trigger: active.trigger }); + }; + + const confirm = async () => { + const active = current.current; + if ( + active?.scope !== scope || + active.kind !== 'open' || + active.status === 'pending' || + deletingConversationIds.includes(active.target.id) + ) + return; + + const pending: DeleteInteraction = { ...active, status: 'pending' }; + transition(pending); + + const removed = await onDelete(active.target.id); + if (current.current !== pending) return; + + if (removed) transition({ kind: 'closed', scope, trigger: pending.trigger }); + else transition({ ...pending, status: 'failed' }); + }; + + const active = interaction?.scope === scope && interaction.kind === 'open' ? interaction : null; + + return { + target: active?.target ?? null, + deleting: !!active && (active.status === 'pending' || deletingConversationIds.includes(active.target.id)), + error: active?.status === 'failed' ? (error ?? 'Could not delete the conversation.') : undefined, + request, + cancel, + confirm + }; +}; diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.css.ts b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.css.ts new file mode 100644 index 00000000000..fab9fcb97b4 --- /dev/null +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.css.ts @@ -0,0 +1,196 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { style } from '@vanilla-extract/css'; +import { vars } from '@/shared/ui/assistant-theme/AssistantScope.css'; + +export const panel = style({ + position: 'relative', + display: 'flex', + flex: 1, + minWidth: 0, + minHeight: 0, + height: '100%', + overflow: 'hidden', + color: vars.text, + background: vars.bg +}); + +export const content = style({ + display: 'flex', + flex: 1, + minWidth: 0, + minHeight: 0 +}); + +export const conversation = style({ + display: 'flex', + flexDirection: 'column', + flex: 1, + minWidth: 0, + minHeight: 0 +}); + +export const hidden = style({ display: 'none' }); + +export const messages = style({ + flex: '1 1 0', + minWidth: 0, + minHeight: 0, + overflowY: 'auto', + overflowX: 'hidden', + overflowAnchor: 'none', + overscrollBehavior: 'contain', + scrollbarGutter: 'stable', + padding: `${vars.space4} ${vars.space3} ${vars.space2}` +}); + +export const messageStack = style({ + display: 'flex', + flexDirection: 'column', + gap: vars.space4, + minWidth: 0 +}); + +export const messageRow = style({ + display: 'flex', + flexDirection: 'column', + minWidth: 0 +}); + +export const statusArea = style({ + flexShrink: 1, + minHeight: 0, + maxHeight: '40%', + overflowY: 'auto', + overscrollBehavior: 'contain' +}); + +export const note = style({ + flexShrink: 0, + margin: 0, + padding: `${vars.space2} ${vars.space3}`, + color: vars.textSecondary, + fontSize: vars.fontSizeSm, + overflowWrap: 'anywhere' +}); + +export const empty = style({ + margin: 0, + padding: `${vars.space5} ${vars.space3}`, + color: vars.textSecondary, + textAlign: 'center' +}); + +export const warning = style({ + flexShrink: 0, + margin: vars.space2, + padding: vars.space2, + color: vars.text, + background: vars.fill, + border: `1px solid ${vars.borderStrong}`, + borderRadius: vars.radiusSm, + fontSize: vars.fontSizeSm, + overflowWrap: 'anywhere' +}); + +export const composer = style({ + flexShrink: 0, + minWidth: 0, + margin: 0, + padding: 0, + border: 0 +}); + +export const listView = style({ + flex: 1, + minWidth: 0, + minHeight: 0, + overflowY: 'auto', + overflowX: 'hidden', + overscrollBehavior: 'contain', + padding: vars.space2 +}); + +export const listNavigation = style({ paddingBottom: vars.space2 }); + +export const action = style({ + minHeight: 30, + maxWidth: '100%', + padding: `${vars.space1} ${vars.space2}`, + border: `1px solid ${vars.borderStrong}`, + borderRadius: vars.radiusSm, + color: vars.text, + background: vars.bg, + font: 'inherit', + fontSize: vars.fontSizeSm, + lineHeight: vars.lineHeight, + cursor: 'pointer', + overflowWrap: 'anywhere', + selectors: { + '&:hover:not(:disabled)': { background: vars.fill }, + '&:disabled': { cursor: 'not-allowed', opacity: 0.6 } + } +}); + +export const primaryAction = style([ + action, + { + color: vars.accentText, + borderColor: vars.accentBorder, + background: vars.accentBg + } +]); + +export const confirmationBackdrop = style({ + position: 'absolute', + inset: 0, + zIndex: 1, + display: 'flex', + alignItems: 'center', + justifyContent: 'center', + padding: vars.space3, + background: vars.bg, + overflowY: 'auto', + overscrollBehavior: 'contain' +}); + +export const confirmation = style({ + width: '100%', + maxWidth: 360, + maxHeight: '100%', + overflowY: 'auto', + padding: vars.space3, + border: `1px solid ${vars.borderStrong}`, + borderRadius: vars.radius, + background: vars.bg, + boxShadow: vars.shadow, + overflowWrap: 'anywhere' +}); + +export const confirmationTitle = style({ + margin: `0 0 ${vars.space2}`, + fontSize: vars.fontSize, + color: vars.text +}); + +export const confirmationDescription = style({ + margin: `0 0 ${vars.space3}`, + color: vars.textSecondary +}); + +export const confirmationActions = style({ + display: 'flex', + flexWrap: 'wrap', + justifyContent: 'flex-end', + gap: vars.space2 +}); diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.spec.tsx new file mode 100644 index 00000000000..d72d4878f87 --- /dev/null +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.spec.tsx @@ -0,0 +1,836 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { act, fireEvent, render, screen, within } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; +import { AssistantConnectionError, type AssistantRunEvent } from '@/entities/assistant/model/assistantContract'; +import { createAssistantFixture, type AssistantFixtureScenario } from '../fixtures/assistantFixture'; +import { ASSISTANT_HISTORY_CONVERSATIONS, FIXTURE_NOW } from '../fixtures/assistantFixtureData'; +import type { AssistantDataSource } from '../model/assistantDataSource'; +import { AssistantConversationPanel } from './AssistantConversationPanel'; + +const flush = async () => { + await act(async () => {}); +}; + +const deferred = () => { + let resolve!: () => void; + const promise = new Promise(accept => { + resolve = accept; + }); + + return { promise, resolve }; +}; + +const input = () => screen.getByRole('textbox', { name: 'Question' }) as HTMLTextAreaElement; +const send = () => screen.getByRole('button', { name: 'Send' }) as HTMLButtonElement; +const listTrigger = () => screen.getByRole('button', { name: 'Open conversations' }); +const messages = () => screen.getByRole('region', { name: 'Conversation messages' }); + +const mountPanel = async (source: AssistantDataSource, onKeyDown = vi.fn()) => { + render( +
+ +
+ ); + await flush(); + return { onKeyDown }; +}; + +const select = async (title: string) => { + fireEvent.click(listTrigger()); + fireEvent.click(screen.getByRole('button', { name: new RegExp(`^${title}`) })); + await flush(); +}; + +const submit = async (question: string) => { + fireEvent.change(input(), { target: { value: question } }); + fireEvent.click(send()); + await flush(); +}; + +const fixture = (scenario?: AssistantFixtureScenario) => + createAssistantFixture({ conversations: ASSISTANT_HISTORY_CONVERSATIONS, now: FIXTURE_NOW, scenario, delayMs: 0 }); + +const streamingFixture = () => { + const source = fixture(); + const firstDelta = deferred(); + const secondDelta = deferred(); + const finish = deferred(); + const run = vi.fn(async function* (): AsyncIterable { + yield { type: 'run.started' }; + await firstDelta.promise; + yield { type: 'message.delta', messageId: 'controlled-answer', delta: 'Only revenue A' }; + await secondDelta.promise; + yield { type: 'message.delta', messageId: 'controlled-answer', delta: ' continues' }; + await finish.promise; + yield { type: 'run.failed', message: 'Controlled run stopped.' }; + }); + + return { source: { ...source, run }, run, firstDelta, secondDelta, finish }; +}; + +describe('AssistantConversationPanel', () => { + it('opens saved conversations and restores the trigger after select, Back and Escape without closing the shell', async () => { + const source = fixture(); + const getMessages = vi.spyOn(source.api, 'getMessages'); + const { onKeyDown } = await mountPanel(source); + + await select('Revenue analysis'); + + expect(screen.getByText('Revenue increased in all three regions, led by Europe.')).toBeTruthy(); + expect(document.activeElement).toBe(listTrigger()); + + fireEvent.change(input(), { target: { value: 'Keep my selected draft' } }); + + fireEvent.click(listTrigger()); + + expect(screen.getByRole('button', { name: /^Revenue analysis/ }).getAttribute('aria-current')).toBe('true'); + + fireEvent.click(screen.getByRole('button', { name: 'Back' })); + + expect(input().value).toBe('Keep my selected draft'); + expect(document.activeElement).toBe(listTrigger()); + + fireEvent.click(listTrigger()); + const search = screen.getByRole('textbox', { name: 'Search conversations' }); + search.focus(); + fireEvent.keyDown(search, { key: 'Escape' }); + + expect(screen.queryByRole('textbox', { name: 'Search conversations' })).toBeNull(); + expect(input().value).toBe('Keep my selected draft'); + expect(document.activeElement).toBe(listTrigger()); + expect(onKeyDown).not.toHaveBeenCalled(); + expect(getMessages.mock.calls).toEqual([['c1']]); + + fireEvent.keyDown(input(), { key: 'ArrowLeft' }); + + expect(onKeyDown).toHaveBeenCalledTimes(1); + }); + + it('renders a saved answer whose opaque id matches an object property', async () => { + const source = fixture(); + vi.spyOn(source.api, 'getMessages').mockResolvedValueOnce({ + messages: [ + { + id: 'constructor', + role: 'assistant', + segments: [{ id: 'saved-segment', content: 'This saved answer must remain visible.' }] + } + ], + cursor: null + }); + await mountPanel(source); + + await select('Revenue analysis'); + + expect(within(messages()).getByText('This saved answer must remain visible.')).toBeTruthy(); + }); + + it('loads earlier history after a rejected send supersedes its pending page request', async () => { + // Make the viewport eligible for automatic history loading. + const height = vi.spyOn(Element.prototype, 'clientHeight', 'get').mockReturnValue(100); + const width = vi.spyOn(Element.prototype, 'clientWidth', 'get').mockReturnValue(100); + const earlier = deferred(); + + try { + const source = fixture('send-rejection'); + const getMessages = source.api.getMessages; + let pending = true; + vi.spyOn(source.api, 'getMessages').mockImplementation(async (id, before) => { + if (before && pending) { + pending = false; + await earlier.promise; + } + return getMessages(id, before); + }); + await mountPanel(source); + + await select('Revenue analysis'); + + expect(source.api.getMessages).toHaveBeenCalledWith('c1', 'c1:earlier'); + + await submit('A question rejected before acceptance'); + await act(async () => earlier.resolve()); + + expect(await within(messages()).findByText('I checked the regional revenue paragraph.')).toBeTruthy(); + expect(input().value).toBe('A question rejected before acceptance'); + } finally { + earlier.resolve(); + height.mockRestore(); + width.mockRestore(); + } + }); + + it('keeps initial and repeated New local, preserving its draft until the first send', async () => { + const source = fixture(); + const create = vi.spyOn(source.api, 'createConversation'); + const run = vi.spyOn(source, 'run'); + await mountPanel(source); + + fireEvent.click(screen.getByRole('button', { name: 'New conversation' })); + fireEvent.change(input(), { target: { value: 'Local draft question' } }); + await select('Notebook summary'); + fireEvent.click(screen.getByRole('button', { name: 'New conversation' })); + + expect(input().value).toBe('Local draft question'); + + fireEvent.click(screen.getByRole('button', { name: 'New conversation' })); + + expect(create).not.toHaveBeenCalled(); + expect(run).not.toHaveBeenCalled(); + + fireEvent.click(send()); + await flush(); + + expect(create).toHaveBeenCalledTimes(1); + expect(run).toHaveBeenCalledWith('c4', 'Local draft question', expect.any(AbortSignal)); + expect(screen.getAllByRole('article', { name: 'Your question' })).toHaveLength(1); + expect( + within(screen.getByRole('article', { name: 'Your question' })).getByText('Local draft question') + ).toBeTruthy(); + expect(screen.getAllByRole('article', { name: 'Assistant answer' })).toHaveLength(1); + expect(screen.getByRole('table')).toBeTruthy(); + expect(screen.getByText('12%', { selector: 'strong' })).toBeTruthy(); + }); + + it('disables only New during creation, including the list, and enables it before the accepted run finishes', async () => { + const source = fixture(); + const creation = deferred(); + const acceptance = deferred(); + const create = source.api.createConversation; + source.api.createConversation = async body => { + await creation.promise; + return create(body); + }; + source.run = async function* () { + await acceptance.promise; + yield { type: 'run.started' }; + yield { type: 'run.failed', message: 'Controlled completion.' }; + }; + await mountPanel(source); + + await submit('Create this draft'); + + expect(screen.getByRole('button', { name: 'New conversation' }).matches(':disabled')).toBe(true); + + await select('Notebook summary'); + + expect(input().disabled).toBe(false); + + fireEvent.change(input(), { target: { value: 'Existing conversation remains editable' } }); + + expect(send().matches(':disabled')).toBe(false); + expect(screen.getByRole('button', { name: 'New conversation' }).matches(':disabled')).toBe(true); + + fireEvent.click(listTrigger()); + + expect(screen.getByRole('button', { name: 'New conversation' }).matches(':disabled')).toBe(true); + + await act(async () => creation.resolve()); + + expect(screen.getByRole('button', { name: 'New conversation' }).matches(':disabled')).toBe(false); + + fireEvent.click(screen.getByRole('button', { name: 'Back' })); + + expect(input().value).toBe('Existing conversation remains editable'); + + fireEvent.click(screen.getByRole('button', { name: 'New conversation' })); + + expect(input().value).toBe(''); + + await act(async () => acceptance.resolve()); + }); + + it('preserves question selection and expanded tool focus through completion and saved history', async () => { + const source = fixture(); + const completion = deferred(); + const history = deferred(); + const originalRun = source.run; + const originalGetMessages = source.api.getMessages; + source.run = async function* (id, content, signal) { + for await (const event of originalRun(id, content, signal)) { + if (event.type === 'run.completed') await completion.promise; + yield event; + } + }; + vi.spyOn(source.api, 'getMessages').mockImplementationOnce(async id => { + await history.promise; + return originalGetMessages(id); + }); + await mountPanel(source); + + await submit('Keep this question selected'); + + const summary = screen.getByText('Used 1 tool'); + summary.closest('details')!.open = true; + summary.focus(); + const selection = window.getSelection()!; + const range = document.createRange(); + range.selectNodeContents(screen.getByText('Keep this question selected')); + selection.removeAllRanges(); + selection.addRange(range); + + await act(async () => completion.resolve()); + + expect(within(messages()).queryByText('Loading conversation…')).toBeNull(); + expect(screen.getByText('Used 1 tool').closest('details')!.open).toBe(true); + expect(document.activeElement).toBe(summary); + expect(selection.toString()).toBe('Keep this question selected'); + + await act(async () => history.resolve()); + + expect(screen.getByText('Used 1 tool').closest('details')!.open).toBe(true); + expect(document.activeElement).toBe(summary); + expect(selection.toString()).toBe('Keep this question selected'); + expect( + within(messages()) + .getAllByRole('article') + .map(article => article.getAttribute('aria-label')) + ).toEqual(['Your question', 'Assistant answer']); + + selection.removeAllRanges(); + }); + + it('keeps an earlier identical question selected when another send overtakes its history refresh', async () => { + const source = fixture(); + const history = deferred(); + const originalGetMessages = source.api.getMessages; + vi.spyOn(source.api, 'getMessages').mockImplementationOnce(async id => { + await history.promise; + return originalGetMessages(id); + }); + await mountPanel(source); + await submit('Repeated question'); + + const summary = screen.getByText('Used 1 tool'); + summary.closest('details')!.open = true; + const selection = window.getSelection()!; + const range = document.createRange(); + range.selectNodeContents(screen.getByText('Repeated question')); + selection.removeAllRanges(); + selection.addRange(range); + + try { + await submit('Repeated question'); + + expect(within(messages()).getAllByText('Repeated question')).toHaveLength(2); + expect(selection.toString()).toBe('Repeated question'); + expect(within(messages()).getAllByText('Used 1 tool')[0].closest('details')!.open).toBe(true); + + await act(async () => history.resolve()); + + expect(within(messages()).getAllByText('Repeated question')).toHaveLength(2); + expect(selection.toString()).toBe('Repeated question'); + } finally { + await act(async () => history.resolve()); + selection.removeAllRanges(); + } + }); + + it('cancels deletion without calling the API, and removes the real fixture conversation only after confirmation', async () => { + const source = fixture(); + const remove = vi.spyOn(source.api, 'deleteConversation'); + await mountPanel(source); + await select('Revenue analysis'); + + fireEvent.click(screen.getByRole('button', { name: 'Delete Revenue analysis' })); + + const dialog = screen.getByRole('dialog', { name: 'Delete conversation' }); + expect(remove).not.toHaveBeenCalled(); + + fireEvent.click(within(dialog).getByRole('button', { name: 'Cancel' })); + + expect(screen.queryByRole('dialog')).toBeNull(); + expect(remove).not.toHaveBeenCalled(); + expect(screen.getByText('Revenue increased in all three regions, led by Europe.')).toBeTruthy(); + + fireEvent.click(screen.getByRole('button', { name: 'Delete Revenue analysis' })); + fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Delete conversation' })); + await flush(); + + expect(remove).toHaveBeenCalledExactlyOnceWith('c1'); + expect(screen.queryByRole('dialog')).toBeNull(); + expect(screen.queryByText('Revenue increased in all three regions, led by Europe.')).toBeNull(); + + fireEvent.click(listTrigger()); + + expect(screen.queryByRole('button', { name: /^Revenue analysis/ })).toBeNull(); + expect((await source.api.listConversations()).map(item => item.id)).toEqual(['c2', 'c3']); + }); + + it('consumes modal Escape without deleting or bubbling to the shell', async () => { + const source = fixture(); + const remove = vi.spyOn(source.api, 'deleteConversation'); + const { onKeyDown } = await mountPanel(source); + await select('Revenue analysis'); + + fireEvent.click(screen.getByRole('button', { name: 'Delete Revenue analysis' })); + + const cancel = within(screen.getByRole('dialog')).getByRole('button', { name: 'Cancel' }); + expect(document.activeElement).toBe(cancel); + + fireEvent.keyDown(cancel, { key: 'Escape' }); + + expect(screen.queryByRole('dialog')).toBeNull(); + expect(remove).not.toHaveBeenCalled(); + expect(onKeyDown).not.toHaveBeenCalled(); + expect(screen.getByText('Revenue increased in all three regions, led by Europe.')).toBeTruthy(); + }); + + it('shows a delete rejection and retains the conversation rather than pretending it was removed', async () => { + const source = fixture(); + const remove = vi + .spyOn(source.api, 'deleteConversation') + .mockRejectedValueOnce(new Error('Cannot delete this conversation.')); + await mountPanel(source); + + fireEvent.click(listTrigger()); + fireEvent.click(screen.getByRole('button', { name: 'Delete Revenue analysis' })); + fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Delete conversation' })); + await flush(); + + expect(remove).toHaveBeenCalledExactlyOnceWith('c1'); + expect( + screen.getAllByRole('alert').some(alert => alert.textContent?.includes('Cannot delete this conversation.')) + ).toBe(true); + expect(screen.getByRole('button', { name: /^Revenue analysis/ })).toBeTruthy(); + expect((await source.api.listConversations()).some(item => item.id === 'c1')).toBe(true); + }); + + it('shows create failure, retains the draft, and retries creation only on an explicit send', async () => { + const source = fixture('create-failure'); + const create = vi.spyOn(source.api, 'createConversation'); + const run = vi.spyOn(source, 'run'); + await mountPanel(source); + + await submit('Retain this question'); + + expect(screen.getByRole('alert').textContent).toContain('could not create the conversation'); + expect(input().value).toBe('Retain this question'); + expect(create).toHaveBeenCalledTimes(1); + expect(run).not.toHaveBeenCalled(); + expect(send().matches(':disabled')).toBe(false); + + fireEvent.click(screen.getByRole('button', { name: 'Retry' })); + await flush(); + + expect(create).toHaveBeenCalledTimes(2); + expect(run).toHaveBeenCalledExactlyOnceWith('c4', 'Retain this question', expect.any(AbortSignal)); + expect(screen.queryByRole('alert')).toBeNull(); + }); + + it('retries a rejected first send using its confirmed conversation id without creating another conversation', async () => { + const source = fixture('send-rejection'); + const create = vi.spyOn(source.api, 'createConversation'); + const run = vi.spyOn(source, 'run'); + await mountPanel(source); + + await submit('Retry the same conversation'); + + expect(screen.getByRole('alert').textContent).toContain('could not accept this message'); + expect(input().value).toBe('Retry the same conversation'); + expect(send().matches(':disabled')).toBe(false); + + fireEvent.click(screen.getByRole('button', { name: 'Retry' })); + await flush(); + + expect(create).toHaveBeenCalledTimes(1); + expect(run.mock.calls.map(([id]) => id)).toEqual(['c4', 'c4']); + expect(screen.getAllByRole('article', { name: 'Your question' })).toHaveLength(1); + expect(screen.queryByRole('alert')).toBeNull(); + }); + + it('keeps an unknown draft editable and reloads only on explicit recovery without automatically resending', async () => { + const source = fixture('unknown'); + const run = vi.spyOn(source, 'run'); + const getMessages = vi.spyOn(source.api, 'getMessages'); + await mountPanel(source); + + await submit('Was this accepted?'); + + expect(input().value).toBe('Was this accepted?'); + expect(input().disabled).toBe(false); + + fireEvent.change(input(), { target: { value: 'My next explicit question' } }); + + expect(input().value).toBe('My next explicit question'); + expect(send().matches(':disabled')).toBe(false); + expect(run).toHaveBeenCalledTimes(1); + + fireEvent.click(screen.getByRole('button', { name: 'Reload conversation' })); + await flush(); + + expect(getMessages).toHaveBeenCalledExactlyOnceWith('c4'); + expect(screen.getByRole('article', { name: 'Your question' }).textContent).toContain('Was this accepted?'); + expect(input().value).toBe('My next explicit question'); + expect(send().matches(':disabled')).toBe(false); + expect(run).toHaveBeenCalledTimes(1); + + fireEvent.click(send()); + await flush(); + + expect(run).toHaveBeenCalledTimes(2); + expect(run.mock.calls[1][0]).toBe('c4'); + expect(run.mock.calls[1][1]).toBe('My next explicit question'); + }); + + it('allows an explicit normal Send immediately after an unknown outcome without a history prerequisite', async () => { + const source = fixture('unknown'); + const run = vi.spyOn(source, 'run'); + const create = vi.spyOn(source.api, 'createConversation'); + const getMessages = vi.spyOn(source.api, 'getMessages'); + await mountPanel(source); + + await submit('First question with unknown acceptance'); + + expect(run).toHaveBeenCalledTimes(1); + expect(getMessages).not.toHaveBeenCalled(); + + fireEvent.change(input(), { target: { value: 'Explicit next question' } }); + + expect(send().matches(':disabled')).toBe(false); + + fireEvent.click(send()); + await flush(); + + expect(create).toHaveBeenCalledTimes(1); + expect(run.mock.calls.map(([id, question]) => [id, question])).toEqual([ + ['c4', 'First question with unknown acceptance'], + ['c4', 'Explicit next question'] + ]); + expect(screen.getByRole('table')).toBeTruthy(); + }); + + it('offers the existing explicit Retry after an accepted failure without requiring a history reload', async () => { + const source = fixture('run-failure'); + const run = vi.spyOn(source, 'run'); + const getMessages = vi.spyOn(source.api, 'getMessages'); + await mountPanel(source); + + await submit('Read the revenue data'); + + expect(screen.getAllByRole('alert').some(alert => alert.textContent?.includes('stopped while reading'))).toBe(true); + expect(input().value).toBe('Read the revenue data'); + expect(send().matches(':disabled')).toBe(false); + expect(run).toHaveBeenCalledTimes(1); + expect(getMessages).not.toHaveBeenCalled(); + + fireEvent.click(screen.getByRole('button', { name: 'Retry' })); + await flush(); + + expect(run).toHaveBeenCalledTimes(2); + expect(run.mock.calls.map(([id]) => id)).toEqual(['c4', 'c4']); + }); + + it('keeps a late A stream out of B and returns to A without starting a second consumer', async () => { + const harness = streamingFixture(); + const getMessages = vi.spyOn(harness.source.api, 'getMessages'); + await mountPanel(harness.source); + await select('Revenue analysis'); + await submit('Question for A'); + await select('Notebook summary'); + fireEvent.change(input(), { target: { value: 'Question for B' } }); + input().focus(); + const focusedComposer = input(); + + await act(async () => harness.firstDelta.resolve()); + + expect(screen.queryByText('Only revenue A')).toBeNull(); + expect(screen.getByText('This notebook compares monthly revenue across regions.')).toBeTruthy(); + expect(input().value).toBe('Question for B'); + expect(document.activeElement).toBe(focusedComposer); + + const loads = getMessages.mock.calls.length; + await select('Revenue analysis'); + + expect(screen.getByText('Only revenue A')).toBeTruthy(); + expect(harness.run).toHaveBeenCalledTimes(1); + expect(getMessages).toHaveBeenCalledTimes(loads); + + await act(async () => { + harness.secondDelta.resolve(); + harness.finish.resolve(); + }); + }); + + it('disables the composer for read-only history without interpreting it as an active run', async () => { + const source = fixture(); + const run = vi.spyOn(source, 'run'); + await mountPanel(source); + + await select('Shared analysis'); + + expect(screen.getByText('This shared conversation can be read, but not changed.')).toBeTruthy(); + expect(input().disabled).toBe(true); + expect(send().matches(':disabled')).toBe(true); + expect(screen.queryByRole('button', { name: 'Delete Shared analysis' })).toBeNull(); + expect(screen.queryByText('Thinking…')).toBeNull(); + + fireEvent.submit(input().form!); + + expect(run).not.toHaveBeenCalled(); + }); + + it('shows a history failure and reloads explicitly without sending a question', async () => { + const source = fixture(); + const getMessages = vi + .spyOn(source.api, 'getMessages') + .mockRejectedValueOnce(new Error('Saved history is unavailable.')); + const run = vi.spyOn(source, 'run'); + await mountPanel(source); + + await select('Revenue analysis'); + + expect(screen.getByRole('alert').textContent).toContain('Saved history is unavailable.'); + + fireEvent.click(screen.getByRole('button', { name: 'Reload conversation' })); + await flush(); + + expect(getMessages.mock.calls).toEqual([['c1'], ['c1']]); + expect(screen.getByText('Revenue increased in all three regions, led by Europe.')).toBeTruthy(); + expect(screen.queryByRole('alert')).toBeNull(); + expect(run).not.toHaveBeenCalled(); + }); + + it('confirms deletion of an active conversation and lets the API decide the result', async () => { + const harness = streamingFixture(); + const remove = vi + .spyOn(harness.source.api, 'deleteConversation') + .mockRejectedValueOnce(new Error('Question active.')); + await mountPanel(harness.source); + await select('Revenue analysis'); + await submit('Keep A running'); + await select('Notebook summary'); + + fireEvent.click(listTrigger()); + fireEvent.click(screen.getByRole('button', { name: 'Delete Revenue analysis' })); + + expect(screen.getByRole('dialog')).toBeTruthy(); + expect(remove).not.toHaveBeenCalled(); + + fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Delete conversation' })); + await flush(); + + expect(remove).toHaveBeenCalledExactlyOnceWith('c1'); + expect(within(screen.getByRole('dialog')).getByRole('alert').textContent).toContain('Question active.'); + + await act(async () => { + harness.firstDelta.resolve(); + harness.secondDelta.resolve(); + harness.finish.resolve(); + }); + }); + + it('discards a confirmation when the source changes rather than applying it to the same id in another source', async () => { + const first = fixture(); + const second = fixture(); + const remove = vi.spyOn(second.api, 'deleteConversation'); + const view = render(); + await flush(); + await select('Revenue analysis'); + + fireEvent.click(screen.getByRole('button', { name: 'Delete Revenue analysis' })); + + expect(screen.getByRole('dialog')).toBeTruthy(); + + view.rerender(); + await flush(); + + expect(screen.queryByRole('dialog')).toBeNull(); + + fireEvent.click(listTrigger()); + + expect(screen.getByRole('button', { name: /^Revenue analysis/ })).toBeTruthy(); + expect(remove).not.toHaveBeenCalled(); + }); + + it('ignores an old deletion continuation without clearing a new source confirmation or its cancel action', async () => { + const first = fixture(); + const second = fixture(); + const gate = deferred(); + const remove = vi.spyOn(second.api, 'deleteConversation'); + const view = render(); + await flush(); + + await submit('Temporary conversation'); + fireEvent.click(screen.getByRole('button', { name: 'Delete New conversation' })); + fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Delete conversation' })); + await flush(); + + await select('Notebook summary'); + fireEvent.click(screen.getByRole('button', { name: 'Delete Notebook summary' })); + fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Delete conversation' })); + await flush(); + + vi.spyOn(first.api, 'deleteConversation').mockImplementationOnce(() => gate.promise); + await select('Revenue analysis'); + fireEvent.click(screen.getByRole('button', { name: 'Delete Revenue analysis' })); + fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Delete conversation' })); + + view.rerender(); + await flush(); + await select('Revenue analysis'); + fireEvent.click(screen.getByRole('button', { name: 'Delete Revenue analysis' })); + + await act(async () => gate.resolve()); + + const dialog = screen.getByRole('dialog'); + expect(within(dialog).queryByRole('alert')).toBeNull(); + const cancel = within(dialog).getByRole('button', { name: 'Cancel' }); + expect(cancel.matches(':disabled')).toBe(false); + + fireEvent.click(cancel); + + expect(screen.queryByRole('dialog')).toBeNull(); + expect(remove).not.toHaveBeenCalled(); + expect(screen.getByText('Revenue increased in all three regions, led by Europe.')).toBeTruthy(); + }); + + it('keeps the draft editable and normal Send available while history is loading', async () => { + const source = fixture(); + const history = deferred(); + const originalGetMessages = source.api.getMessages; + vi.spyOn(source.api, 'getMessages').mockImplementationOnce(async id => { + await history.promise; + return originalGetMessages(id); + }); + const run = vi.spyOn(source, 'run'); + await mountPanel(source); + + await select('Revenue analysis'); + + expect(screen.getByText('Loading conversation…')).toBeTruthy(); + expect(input().disabled).toBe(false); + + fireEvent.change(input(), { target: { value: 'Question during history lookup' } }); + + expect(input().value).toBe('Question during history lookup'); + expect(send().matches(':disabled')).toBe(false); + expect(run).not.toHaveBeenCalled(); + + fireEvent.click(send()); + await flush(); + + expect(run).toHaveBeenCalledExactlyOnceWith('c1', 'Question during history lookup', expect.any(AbortSignal)); + + await act(async () => history.resolve()); + }); + + it.each(['Cancel', 'Escape'])( + 'dismisses a pending delete with %s without cancelling the API or changing a later confirmation', + async dismissal => { + const source = fixture(); + const gate = deferred(); + const originalDelete = source.api.deleteConversation; + const remove = vi.spyOn(source.api, 'deleteConversation').mockImplementationOnce(async id => { + await gate.promise; + return originalDelete(id); + }); + await mountPanel(source); + await select('Revenue analysis'); + + fireEvent.click(screen.getByRole('button', { name: 'Delete Revenue analysis' })); + const confirm = within(screen.getByRole('dialog')).getByRole('button', { name: 'Delete conversation' }); + fireEvent.click(confirm); + fireEvent.click(confirm); + + expect(remove).toHaveBeenCalledExactlyOnceWith('c1'); + + const cancel = within(screen.getByRole('dialog')).getByRole('button', { name: 'Cancel' }); + if (dismissal === 'Escape') fireEvent.keyDown(cancel, { key: 'Escape' }); + else fireEvent.click(cancel); + + expect(screen.queryByRole('dialog')).toBeNull(); + + await select('Notebook summary'); + fireEvent.click(screen.getByRole('button', { name: 'Delete Notebook summary' })); + + await act(async () => gate.resolve()); + + const nextDialog = screen.getByRole('dialog'); + expect(within(nextDialog).getByText(/Delete “Notebook summary”/)).toBeTruthy(); + expect(within(nextDialog).queryByRole('alert')).toBeNull(); + expect(within(nextDialog).getByRole('button', { name: 'Delete conversation' }).matches(':disabled')).toBe(false); + + fireEvent.click(within(nextDialog).getByRole('button', { name: 'Delete conversation' })); + await flush(); + + expect(remove.mock.calls).toEqual([['c1'], ['c2']]); + expect(screen.queryByRole('dialog')).toBeNull(); + + fireEvent.click(listTrigger()); + + expect(screen.queryByRole('button', { name: /^Revenue analysis/ })).toBeNull(); + expect(screen.queryByRole('button', { name: /^Notebook summary/ })).toBeNull(); + } + ); + + it('closes a reopened confirmation when its pending deletion succeeds for the same conversation', async () => { + const source = fixture(); + const gate = deferred(); + const originalDelete = source.api.deleteConversation; + const remove = vi.spyOn(source.api, 'deleteConversation').mockImplementationOnce(async id => { + await gate.promise; + return originalDelete(id); + }); + await mountPanel(source); + await select('Revenue analysis'); + + fireEvent.click(screen.getByRole('button', { name: 'Delete Revenue analysis' })); + fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Delete conversation' })); + fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Cancel' })); + fireEvent.click(screen.getByRole('button', { name: 'Delete Revenue analysis' })); + + expect(screen.getByRole('dialog')).toBeTruthy(); + + await act(async () => gate.resolve()); + + expect(screen.queryByRole('dialog')).toBeNull(); + expect(remove).toHaveBeenCalledExactlyOnceWith('c1'); + + fireEvent.click(listTrigger()); + + expect(screen.queryByRole('button', { name: /^Revenue analysis/ })).toBeNull(); + }); + + it.each(['failed', 'connection'])( + 'retains the previous %s answer outcome when the next question is rejected', + async outcome => { + const source = fixture(); + let first = true; + const failure = outcome === 'failed' ? 'The first run failed.' : new AssistantConnectionError().message; + source.run = async function* (): AsyncIterable { + if (!first) { + yield { type: 'run.failed', message: 'The next question was rejected.' }; + return; + } + first = false; + yield { type: 'run.started' }; + yield { type: 'message.delta', messageId: 'partial-answer', delta: 'Partial answer' }; + yield { type: 'tool_call.started', toolCallId: 'unfinished-tool', name: 'read_notebook' }; + if (outcome === 'connection') throw new AssistantConnectionError(); + yield { type: 'run.failed', message: failure }; + }; + await mountPanel(source); + await select('Revenue analysis'); + await submit('First question'); + + await submit('Next question'); + + const retained = screen.getByText('Partial answer').closest('article')!; + expect(within(retained).getByText(/status unknown/)).toBeTruthy(); + expect(within(retained).getByRole('alert').textContent).toContain(failure); + expect(screen.getByText('The next question was rejected.')).toBeTruthy(); + } + ); +}); diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.stories.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.stories.tsx new file mode 100644 index 00000000000..63d2af818f7 --- /dev/null +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.stories.tsx @@ -0,0 +1,236 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { useMemo } from 'react'; +import type { Meta, StoryObj } from '@storybook/react-vite'; +import { expect, userEvent, waitFor, within } from 'storybook/test'; +import { createAssistantFixture, type AssistantFixtureScenario } from '../fixtures/assistantFixture'; +import { + ASSISTANT_DEMO_CONVERSATIONS, + ASSISTANT_HISTORY_CONVERSATIONS, + FIXTURE_NOW +} from '../fixtures/assistantFixtureData'; +import { AssistantConversationPanel } from './AssistantConversationPanel'; + +type PanelDemonstration = + | 'ask' + | 'completed-answer' + | 'two-page-history' + | 'conversation-switching' + | 'creation-failure' + | 'send-rejected' + | 'acceptance-unknown' + | 'accepted-run-failure'; + +const failureScenarios: Partial> = { + 'creation-failure': 'create-failure', + 'send-rejected': 'send-rejection', + 'acceptance-unknown': 'unknown', + 'accepted-run-failure': 'run-failure' +}; + +const FixturePanel = ({ demonstration }: { demonstration: PanelDemonstration }) => { + const source = useMemo( + () => + createAssistantFixture({ + conversations: + demonstration === 'two-page-history' ? ASSISTANT_HISTORY_CONVERSATIONS : ASSISTANT_DEMO_CONVERSATIONS, + now: FIXTURE_NOW, + scenario: failureScenarios[demonstration] ?? 'normal', + delayMs: demonstration === 'conversation-switching' ? 500 : 120 + }), + [demonstration] + ); + + return ( +
+ +
+ ); +}; + +const meta = { + title: 'Assistant/ConversationPanel', + component: FixturePanel, + args: { demonstration: 'ask' }, + argTypes: { demonstration: { control: false, table: { disable: true } } } +} satisfies Meta; +export default meta; +type Story = StoryObj; + +const ask = async (canvasElement: HTMLElement, question: string) => { + const canvas = within(canvasElement); + + await userEvent.type(canvas.getByRole('textbox', { name: 'Question' }), question); + await userEvent.click(canvas.getByRole('button', { name: 'Send' })); + + return canvas; +}; + +const selectConversation = async (canvasElement: HTMLElement, title: RegExp) => { + const canvas = within(canvasElement); + + await userEvent.click(canvas.getByRole('button', { name: 'Open conversations' })); + await userEvent.click(await canvas.findByRole('button', { name: title })); +}; + +export const AskInThePanel: Story = { + parameters: { + docs: { + description: { + story: + 'Manual interactive demo with compact saved conversations. Type a question and Send to create a conversation and stream an answer. New stays local until the first send.' + } + } + } +}; + +export const CompletedAnswer: Story = { + args: { demonstration: 'completed-answer' }, + parameters: { + docs: { + description: { + story: 'Automatically asks a question and waits for the completed Markdown answer and table.' + } + } + }, + play: async ({ canvasElement }) => { + const canvas = await ask(canvasElement, 'Explain the revenue results.'); + const table = await canvas.findByRole('table', {}, { timeout: 10000 }); + const answer = table.closest('article')!; + await waitFor(() => expect(answer).not.toHaveAttribute('aria-busy', 'true'), { timeout: 10000 }); + await expect(table).toBeVisible(); + } +}; + +export const TwoPageHistory: Story = { + args: { demonstration: 'two-page-history' }, + parameters: { + docs: { + description: { + story: + 'Automatically opens Revenue analysis and scrolls its message viewport to the top to load the older page. The earlier question and both parts of the boundary-split answer are then shown together.' + } + } + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await selectConversation(canvasElement, /^Revenue analysis/); + await canvas.findByText('Revenue increased in all three regions, led by Europe.'); + + const viewport = canvas.getByRole('region', { name: 'Conversation messages' }); + await waitFor(() => expect(viewport).not.toHaveAttribute('aria-busy', 'true')); + viewport.scrollTo({ top: 0 }); + await expect(await canvas.findByText('How did revenue change by region?', {}, { timeout: 10000 })).toBeVisible(); + + const earlierPart = await canvas.findByText('I checked the regional revenue paragraph.'); + const joinedAnswer = earlierPart.closest('article')!; + await expect(earlierPart).toBeVisible(); + await expect( + within(joinedAnswer).getByText('Revenue increased in all three regions, led by Europe.') + ).toBeVisible(); + } +}; + +export const ConversationSwitching: Story = { + args: { demonstration: 'conversation-switching' }, + parameters: { + docs: { + description: { + story: + 'Sends in Revenue analysis, switches to Notebook summary during the answer, then returns to the completed result.' + } + } + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await selectConversation(canvasElement, /^Revenue analysis/); + await canvas.findByText('Compare Europe’s order volume with its average order value.'); + + await ask(canvasElement, 'Explain revenue while I check the notebook summary.'); + await canvas.findByText('Thinking…', {}, { timeout: 10000 }); + + await selectConversation(canvasElement, /^Notebook summary/); + await expect(await canvas.findByText('This notebook compares monthly revenue across regions.')).toBeVisible(); + + await selectConversation(canvasElement, /^Revenue analysis/); + await expect(await canvas.findByText('Explain revenue while I check the notebook summary.')).toBeVisible(); + const table = await canvas.findByRole('table', {}, { timeout: 10000 }); + await waitFor(() => expect(table.closest('article')).not.toHaveAttribute('aria-busy', 'true'), { timeout: 10000 }); + await expect(table).toBeVisible(); + } +}; + +export const CreationFailure: Story = { + args: { demonstration: 'creation-failure' }, + parameters: { + docs: { + description: { + story: 'Submits a first question and stops at conversation creation failure, ready for an explicit retry.' + } + } + }, + play: async ({ canvasElement }) => { + const canvas = await ask(canvasElement, 'Keep this question if creation fails.'); + await expect(await canvas.findByRole('alert', {}, { timeout: 10000 })).toBeVisible(); + } +}; + +export const SendRejectedAfterCreation: Story = { + args: { demonstration: 'send-rejected' }, + parameters: { + docs: { + description: { + story: + 'Creates a conversation and stops when its first send is rejected before acceptance. No retry is submitted.' + } + } + }, + play: async ({ canvasElement }) => { + const canvas = await ask(canvasElement, 'Keep this question if the send is rejected.'); + await expect(await canvas.findByRole('alert', {}, { timeout: 10000 })).toBeVisible(); + } +}; + +export const AcceptanceUnknown: Story = { + args: { demonstration: 'acceptance-unknown' }, + parameters: { + docs: { + description: { + story: + 'Stops at unknown acceptance, showing the warning and explicit Reload action without reloading or resending.' + } + } + }, + play: async ({ canvasElement }) => { + const canvas = await ask(canvasElement, 'Was this question accepted?'); + await expect(await canvas.findByRole('button', { name: 'Reload conversation' }, { timeout: 10000 })).toBeVisible(); + await expect(canvas.getAllByRole('alert')[0]).toBeVisible(); + } +}; + +export const AcceptedRunFailure: Story = { + args: { demonstration: 'accepted-run-failure' }, + parameters: { + docs: { + description: { + story: 'Stops after an accepted run fails, leaving the accepted question and inline answer failure visible.' + } + } + }, + play: async ({ canvasElement }) => { + const canvas = await ask(canvasElement, 'Read the revenue data.'); + await expect(await canvas.findByRole('article', { name: 'Your question' })).toBeVisible(); + const answer = await canvas.findByRole('article', { name: 'Assistant answer' }); + await expect(await within(answer).findByRole('alert', {}, { timeout: 10000 })).toBeVisible(); + } +}; diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.tsx new file mode 100644 index 00000000000..155a82cdd61 --- /dev/null +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.tsx @@ -0,0 +1,200 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { useCallback, useLayoutEffect, useRef, useState } from 'react'; +import { AssistantAnswerError } from '@/entities/assistant/ui/AssistantAnswerError'; +import { AssistantComposer } from '@/entities/assistant/ui/AssistantComposer'; +import { ConversationHeader } from '@/entities/assistant/ui/ConversationHeader'; +import { ConversationList } from '@/entities/assistant/ui/ConversationList'; +import { useDeleteConfirmation } from '../lib/useDeleteConfirmation'; +import type { AssistantDataSource } from '../model/assistantDataSource'; +import { useAssistantPanel } from '../model/useAssistantPanel'; +import { ConversationMessages } from './ConversationMessages'; +import { DeleteConversationDialog } from './DeleteConversationDialog'; +import * as styles from './AssistantConversationPanel.css'; + +export interface AssistantConversationPanelProps { + source: AssistantDataSource; + now?: number; +} + +const ignoreSearchChange = () => undefined; + +export const AssistantConversationPanel = ({ source, now }: AssistantConversationPanelProps) => { + const panel = useAssistantPanel(source); + const { entry } = panel; + + const [listOpen, setListOpen] = useState(false); + const listTrigger = useRef(null); + const back = useRef(null); + const navigation = useRef({ scope: source, open: false }); + + const fallbackFocus = useCallback(() => (listOpen ? back.current : listTrigger.current), [listOpen]); + const confirmation = useDeleteConfirmation({ + scope: source, + conversations: panel.conversations, + listLoading: panel.listLoading, + deletingConversationIds: panel.deletingConversationIds, + error: panel.listError, + onDelete: panel.deleteConversation, + fallbackFocus + }); + + useLayoutEffect(() => { + if (navigation.current.scope !== source) { + navigation.current = { scope: source, open: false }; + setListOpen(false); + return; + } + + if (confirmation.target) return; + + const wasOpen = navigation.current.open; + navigation.current.open = listOpen; + if (listOpen && !wasOpen) back.current?.focus(); + else if (!listOpen && wasOpen) listTrigger.current?.focus(); + }, [source, listOpen, confirmation.target]); + + const closeList = () => setListOpen(false); + + const createDraft = () => { + panel.newConversation(); + closeList(); + }; + + const busy = entry.phase === 'creating' || entry.phase === 'pending' || entry.phase === 'running'; + + const recoveryActions = { + send: () => { + void panel.send(); + }, + reload: () => { + void panel.reload(); + } + }; + const retry = entry.failure?.recovery ? recoveryActions[entry.failure.recovery] : undefined; + const answerRetry = entry.answerRecovery ? recoveryActions[entry.answerRecovery] : undefined; + + return ( +
{ + if (listOpen && event.key === 'Escape') { + event.preventDefault(); + event.stopPropagation(); + closeList(); + } + }} + > +
+ + + {listOpen && ( +
+
+ +
+ {panel.listLoading && ( +

+ Loading conversations… +

+ )} + {panel.listError && ( +

+ {panel.listError} +

+ )} + { + panel.select(id); + closeList(); + }} + onCreate={createDraft} + newDisabled={panel.creatingConversation} + onDelete={confirmation.request} + onSearchTermChange={ignoreSearchChange} + now={now} + /> +
+ )} +
+ + {confirmation.target && ( + { + void confirmation.confirm(); + }} + /> + )} +
+ ); +}; diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/ConversationMessages.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/ConversationMessages.tsx new file mode 100644 index 00000000000..ebad2d4189d --- /dev/null +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/ConversationMessages.tsx @@ -0,0 +1,119 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { useMemo } from 'react'; +import { AssistantAnswer } from '@/entities/assistant/ui/AssistantAnswer'; +import { AssistantMessageItem } from '@/entities/assistant/ui/AssistantMessageItem'; +import { MarkdownAnswer } from '@/entities/assistant/ui/MarkdownAnswer'; +import { useConversationScroll } from '../lib/useConversationScroll'; +import type { AssistantPanelEntry } from '../model/assistantPanelState'; +import * as styles from './AssistantConversationPanel.css'; + +const renderMarkdown = (content: string) => ( +
+ +
+); + +export const ConversationMessages = ({ + scope, + entry, + visible, + loadEarlier, + onRetry +}: { + scope: object; + entry: AssistantPanelEntry; + visible: boolean; + loadEarlier: () => Promise; + onRetry?: () => void; +}) => { + const { messages, messageKeys, localAnswerViews, answer, answerKey } = entry; + + const rows = useMemo( + () => + messages.map(message => ({ + key: messageKeys.get(message.id) ?? message.id, + message + })), + [messages, messageKeys] + ); + + const completedMessage = answer?.status === 'done' ? answer.message : undefined; + const rowSegments = useMemo(() => { + const segments = new Map(); + for (const { key, message } of rows) { + if (message.role === 'assistant') segments.set(key, message.segments); + } + + if (answerKey && completedMessage) segments.set(answerKey, completedMessage.segments); + return segments; + }, [rows, answerKey, completedMessage]); + + const { viewport, content, onScroll } = useConversationScroll({ + scope, + conversationKey: entry.key, + rowSegments, + visible, + loadEarlier + }); + + const messageItems = rows.map(({ key, message }) => ( +
+ {message.role === 'user' ? ( + + ) : ( + + )} +
+ )); + + if (answer) { + messageItems.push( +
+ +
+ ); + } + + return ( +
+
+ {entry.loadingEarlier && ( +

+ Loading earlier messages… +

+ )} + {entry.loading && messages.length === 0 && !answer && ( +

+ Loading conversation… +

+ )} + {!entry.loading && entry.loaded && messages.length === 0 && !answer && ( +

Ask a question about this notebook to start a conversation.

+ )} + {messageItems} +
+
+ ); +}; diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/DeleteConversationDialog.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/DeleteConversationDialog.tsx new file mode 100644 index 00000000000..ae7382d9953 --- /dev/null +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/DeleteConversationDialog.tsx @@ -0,0 +1,106 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { useId, useLayoutEffect, useRef, type KeyboardEvent } from 'react'; +import * as styles from './AssistantConversationPanel.css'; + +interface DeleteConversationDialogProps { + title: string; + deleting: boolean; + error?: string; + onCancel: () => void; + onConfirm: () => void; +} + +export const DeleteConversationDialog = ({ + title, + deleting, + error, + onCancel, + onConfirm +}: DeleteConversationDialogProps) => { + const titleId = useId(); + const descriptionId = useId(); + const dialog = useRef(null); + const cancel = useRef(null); + + useLayoutEffect(() => { + if (deleting) dialog.current?.focus(); + else cancel.current?.focus(); + }, [deleting]); + + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key === 'Escape') { + event.preventDefault(); + event.stopPropagation(); + onCancel(); + } else if (event.key === 'Tab') { + const buttons = dialog.current?.querySelectorAll('button:not(:disabled)'); + if (!buttons?.length) { + event.preventDefault(); + dialog.current?.focus(); + return; + } + + const first = buttons[0]; + const last = buttons[buttons.length - 1]; + if (event.shiftKey && (document.activeElement === first || document.activeElement === dialog.current)) { + event.preventDefault(); + last.focus(); + } else if (!event.shiftKey && document.activeElement === last) { + event.preventDefault(); + first.focus(); + } + } + }; + + return ( +
+
+

+ Delete conversation +

+

+ Delete “{title}”? This conversation and its messages will be removed. +

+ {error && ( +

+ {error} +

+ )} + {deleting && ( +

+ Deleting conversation… +

+ )} +
+ + +
+
+
+ ); +}; From 1b6333dcdc81cf0eef1326455ea02355883bbb26 Mon Sep 17 00:00:00 2001 From: miinhho Date: Wed, 21 Oct 2026 10:03:06 +0900 Subject: [PATCH 04/10] [ZEPPELIN-6748] Cover assistant conversation flows in notebook E2E --- .../e2e/models/notebook-assistant-page.ts | 20 +++ .../assistant/assistant-conversation.spec.ts | 152 ++++++++++++++++++ 2 files changed, 172 insertions(+) create mode 100644 zeppelin-web-angular/e2e/tests/notebook/assistant/assistant-conversation.spec.ts diff --git a/zeppelin-web-angular/e2e/models/notebook-assistant-page.ts b/zeppelin-web-angular/e2e/models/notebook-assistant-page.ts index 0403b0b1854..96410ed67c2 100644 --- a/zeppelin-web-angular/e2e/models/notebook-assistant-page.ts +++ b/zeppelin-web-angular/e2e/models/notebook-assistant-page.ts @@ -17,11 +17,31 @@ export class NotebookAssistantPage extends BasePage { readonly toggleButton: Locator; readonly panel: Locator; readonly closeButton: Locator; + readonly question: Locator; + readonly sendButton: Locator; + readonly messages: Locator; constructor(page: Page) { super(page); this.toggleButton = page.getByRole('button', { name: 'Toggle AI Assistant' }); this.panel = page.getByRole('complementary', { name: 'AI Assistant workspace' }); this.closeButton = page.getByRole('button', { name: 'Close AI Assistant' }); + this.question = this.panel.getByRole('textbox', { name: 'Question' }); + this.sendButton = this.panel.getByRole('button', { name: 'Send', exact: true }); + this.messages = this.panel.getByRole('region', { name: 'Conversation messages' }); + } + + async selectConversation(title: string): Promise { + await this.panel.getByRole('button', { name: 'Open conversations' }).click(); + await this.panel + .getByRole('region', { name: 'Conversations', exact: true }) + .getByRole('button') + .filter({ has: this.page.getByText(title, { exact: true }) }) + .click(); + } + + async sendQuestion(content: string): Promise { + await this.question.fill(content); + await this.sendButton.click(); } } diff --git a/zeppelin-web-angular/e2e/tests/notebook/assistant/assistant-conversation.spec.ts b/zeppelin-web-angular/e2e/tests/notebook/assistant/assistant-conversation.spec.ts new file mode 100644 index 00000000000..0d447b78420 --- /dev/null +++ b/zeppelin-web-angular/e2e/tests/notebook/assistant/assistant-conversation.spec.ts @@ -0,0 +1,152 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { expect, test } from '@playwright/test'; +import { NotebookAssistantPage } from '../../../models/notebook-assistant-page'; +import { addPageAnnotationBeforeEach, createTestNotebook, PAGES, waitForZeppelinReady } from '../../../utils'; + +test.describe('Notebook AI Assistant fixture conversations', () => { + addPageAnnotationBeforeEach(PAGES.WORKSPACE.NOTEBOOK_ASSISTANT); + + let assistant: NotebookAssistantPage; + let noteId: string | undefined; + + test.beforeEach(async ({ page }) => { + noteId = undefined; + assistant = new NotebookAssistantPage(page); + + await page.goto('/#/'); + await waitForZeppelinReady(page); + ({ noteId } = await createTestNotebook(page)); + + await page.goto(`/#/notebook/${noteId}?reactAssistant=true`); + await waitForZeppelinReady(page); + await assistant.toggleButton.click(); + await expect(assistant.question).toBeEditable(); + }); + + test.afterEach(async ({ page }) => { + if (noteId) { + const response = await page.request.delete(`/api/notebook/${noteId}`); + expect(response.ok(), 'Delete the disposable notebook').toBe(true); + } + }); + + test('streams a new answer without remounting on completion and restores its saved history', async () => { + const question = 'Explain the revenue results for this new conversation.'; + + await assistant.sendQuestion(question); + + const answer = assistant.messages.getByRole('article', { name: 'Assistant answer', exact: true }); + await expect(answer).toHaveAttribute('aria-busy', 'true'); + await expect(answer).toContainText('I checked'); + const original = await answer.elementHandle(); + expect(original).not.toBeNull(); + + await expect(answer.getByRole('table')).toBeVisible(); + await expect(answer).not.toHaveAttribute('aria-busy', 'true'); + await expect(assistant.question).toBeEditable(); + await expect(assistant.messages).not.toHaveAttribute('aria-busy', 'true'); + expect(await original!.evaluate(element => element.isConnected)).toBe(true); + await expect(assistant.messages.getByRole('article', { name: 'Your question', exact: true })).toContainText( + question + ); + await expect(answer).toHaveCount(1); + + await assistant.closeButton.click(); + await expect(assistant.panel).toBeHidden(); + await assistant.toggleButton.click(); + await assistant.selectConversation('New conversation'); + + await expect(assistant.messages).not.toHaveAttribute('aria-busy', 'true'); + await expect(assistant.messages.getByRole('article', { name: 'Your question', exact: true })).toContainText( + question + ); + await expect(answer).toHaveCount(1); + await expect(answer.getByRole('table')).toBeVisible(); + }); + + test('keeps B draft and results separate when switching away from an active A run', async () => { + const aQuestion = 'Compare revenue for conversation A.'; + const bQuestion = 'Summarize the regions for conversation B.'; + const summary = 'This notebook compares monthly revenue across regions.'; + + await assistant.selectConversation('Notebook summary'); + await expect(assistant.messages).toContainText(summary); + await assistant.question.fill(bQuestion); + await assistant.selectConversation('Revenue analysis'); + await expect(assistant.question).toBeEditable(); + + await assistant.sendQuestion(aQuestion); + + await expect( + assistant.messages.getByRole('article', { name: 'Assistant answer', exact: true }).last() + ).toHaveAttribute('aria-busy', 'true'); + + await assistant.selectConversation('Notebook summary'); + await expect(assistant.question).toHaveValue(bQuestion); + await expect(assistant.question).toBeEditable(); + await expect(assistant.messages).toContainText(summary); + await expect(assistant.messages).not.toContainText(aQuestion); + + await assistant.sendButton.click(); + const bAnswer = assistant.messages.getByRole('article', { name: 'Assistant answer', exact: true }).last(); + + await expect(bAnswer.getByRole('table')).toBeVisible(); + await expect(bAnswer).not.toHaveAttribute('aria-busy', 'true'); + await expect(assistant.question).toBeEditable(); + await expect(assistant.messages).not.toHaveAttribute('aria-busy', 'true'); + await expect(assistant.messages.getByText(bQuestion, { exact: true })).toHaveCount(1); + await expect(assistant.messages).toContainText(summary); + await expect(assistant.messages).not.toContainText(aQuestion); + + await assistant.question.fill('Keep this next B draft.'); + + await assistant.selectConversation('Revenue analysis'); + await expect(assistant.messages.getByText(aQuestion, { exact: true })).toHaveCount(1); + await expect(assistant.messages.getByRole('table')).toHaveCount(1); + await expect(assistant.question).toBeEditable(); + await expect(assistant.messages).not.toContainText(bQuestion); + + await assistant.selectConversation('Notebook summary'); + + await expect(assistant.question).toHaveValue('Keep this next B draft.'); + await expect(assistant.messages.getByText(bQuestion, { exact: true })).toHaveCount(1); + await expect(assistant.messages.getByRole('table')).toHaveCount(1); + await expect(assistant.messages).toContainText(summary); + }); + + test('scrolling to earlier history joins the split answer without replacing the later answer', async () => { + await assistant.selectConversation('Revenue analysis'); + const laterAnswer = assistant.messages.getByRole('article', { name: 'Assistant answer', exact: true }).filter({ + hasText: 'Compare Europe’s order volume with its average order value.' + }); + await expect(laterAnswer).toBeVisible(); + const original = await laterAnswer.elementHandle(); + expect(original).not.toBeNull(); + + await assistant.messages.evaluate(element => { + element.scrollTop = 0; + }); + + await expect(assistant.messages.getByText('How did revenue change by region?', { exact: true })).toHaveCount(1); + await expect(assistant.messages).not.toHaveAttribute('aria-busy', 'true'); + const joinedAnswer = assistant.messages.getByRole('article', { name: 'Assistant answer', exact: true }).filter({ + hasText: 'I checked the regional revenue paragraph.' + }); + await expect(joinedAnswer).toContainText('Revenue increased in all three regions, led by Europe.'); + await expect(assistant.messages.getByRole('article', { name: 'Assistant answer', exact: true })).toHaveCount(2); + await expect(laterAnswer).toHaveCount(1); + expect(await original!.evaluate(element => element.isConnected)).toBe(true); + await expect(assistant.question).toBeEditable(); + }); +}); From c3fc417c5357b135e16372d659172021a24486ab Mon Sep 17 00:00:00 2001 From: YONGJAE LEE Date: Thu, 22 Oct 2026 01:10:25 +0900 Subject: [PATCH 05/10] [ZEPPELIN-6748] Restore history after rejected sends --- .../model/assistantConversation.ts | 10 +++- .../model/useAssistantPanel.spec.tsx | 58 +++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts index 06c730627a4..3e7a1c1b67e 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts @@ -86,12 +86,12 @@ export class AssistantConversationSession { if (!history.loaded && !history.loading && !this.deleting && this.phase === 'idle') void this.reload(); } - async reload(): Promise { + async reload(preserveFailure = false): Promise { if (this.disposed || !this.conversation || this.deleting || (this.phase !== 'idle' && this.phase !== 'unknown')) { return; } - this.failure = undefined; + if (!preserveFailure) this.failure = undefined; const result = await this.history.loadLatest(this.source.api, this.conversation.id); if (this.disposed || result.status === 'ignored' || !this.history.isCurrent(result)) return; @@ -101,7 +101,7 @@ export class AssistantConversationSession { kind: 'connection', recovery: 'reload' }; - } else if (this.phase !== 'unknown') { + } else if (this.phase !== 'unknown' && !preserveFailure) { this.submittedQuestion = undefined; } @@ -229,6 +229,10 @@ export class AssistantConversationSession { this.publish(); if (run.snapshot.outcome === 'completed') await this.reload(); + else if (!run.snapshot.accepted && !this.history.snapshot.loaded && run.snapshot.outcome === 'failed') { + // A rejected send invalidated the first history read; restore it without losing the retryable question. + await this.reload(true); + } } async delete(): Promise { diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx index edeabe4b1c7..0b7ef437a4c 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx @@ -1443,4 +1443,62 @@ describe('useAssistantPanel', () => { model.dispose(); } ); + + it('restores initial history after a rejected send without losing its error or draft', async () => { + const harness = sourceHarness(); + const initial = deferred(); + const saved: AssistantMessagePage = { + messages: [{ id: 'saved-question', role: 'user', content: 'Existing question' }], + cursor: 'earlier' + }; + harness.api.getMessages.mockImplementationOnce(() => initial.promise).mockResolvedValueOnce(saved); + const panel = await mountPanel(harness.source); + act(() => panel.result.current.select('a')); + const sending = await sendQuestion(panel, 'Retry this question'); + + await act(async () => { + harness.feeds[0].push({ type: 'run.failed', message: 'Message rejected' }); + await sending.completion; + initial.resolve({ messages: [], cursor: null }); + }); + + expect(panel.result.current.entry).toMatchObject({ + loaded: true, + loading: false, + messages: saved.messages, + cursor: 'earlier', + draft: 'Retry this question', + failure: { message: 'Message rejected', recovery: 'send' } + }); + }); + + it('offers history recovery when restoring initial history after rejection fails', async () => { + const harness = sourceHarness(); + const initial = deferred(); + harness.api.getMessages + .mockImplementationOnce(() => initial.promise) + .mockRejectedValueOnce(new Error('History unavailable')); + const panel = await mountPanel(harness.source); + act(() => panel.result.current.select('a')); + const sending = await sendQuestion(panel, 'Keep this question'); + + await act(async () => { + harness.feeds[0].push({ type: 'run.failed', message: 'Message rejected' }); + await sending.completion; + initial.resolve({ messages: [], cursor: null }); + }); + + expect(panel.result.current.entry).toMatchObject({ + draft: 'Keep this question', + failure: { message: 'History unavailable', recovery: 'reload' } + }); + harness.api.getMessages.mockResolvedValueOnce({ + messages: [{ id: 'saved', role: 'user', content: 'Recovered history' }], + cursor: null + }); + await act(async () => panel.result.current.reload()); + expect(panel.result.current.entry.messages).toEqual([{ id: 'saved', role: 'user', content: 'Recovered history' }]); + expect(panel.result.current.entry.draft).toBe('Keep this question'); + }); + }); From 406e075d6a6740e36e5cfbaeb7fcc38dd9450c92 Mon Sep 17 00:00:00 2001 From: YONGJAE LEE Date: Thu, 22 Oct 2026 01:11:11 +0900 Subject: [PATCH 06/10] [ZEPPELIN-6748] Refresh conversation activity after accepted runs --- .../model/assistantConversation.ts | 4 +- .../model/assistantPanelModel.ts | 32 +++++-- .../model/useAssistantPanel.spec.tsx | 94 +++++++++++++++++++ 3 files changed, 122 insertions(+), 8 deletions(-) diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts index 3e7a1c1b67e..f96e626f769 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts @@ -42,7 +42,8 @@ export class AssistantConversationSession { conversation: AssistantConversation | undefined, private readonly source: AssistantDataSource, private readonly changed: () => void, - private readonly confirmed: (session: AssistantConversationSession, conversation: AssistantConversation) => void + private readonly confirmed: (session: AssistantConversationSession, conversation: AssistantConversation) => void, + private readonly activity: () => void ) { this.conversation = conversation; this.history = new AssistantHistory(key, !!conversation, this.publish); @@ -228,6 +229,7 @@ export class AssistantConversationSession { this.phase = run.snapshot.outcome === 'unknown' ? 'unknown' : 'idle'; this.publish(); + if (run.snapshot.accepted) this.activity(); if (run.snapshot.outcome === 'completed') await this.reload(); else if (!run.snapshot.accepted && !this.history.snapshot.loaded && run.snapshot.outcome === 'failed') { // A rejected send invalidated the first history read; restore it without losing the retryable question. diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantPanelModel.ts b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantPanelModel.ts index 8912bd86b96..3f1ce5455f4 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantPanelModel.ts +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantPanelModel.ts @@ -19,6 +19,7 @@ export class AssistantPanelModel { private conversations: AssistantConversation[] = []; private listLoading = true; private listError?: string; + private deletionError?: string; private selectedKey = 'draft:0'; private draftKey: string | undefined = 'draft:0'; @@ -30,6 +31,7 @@ export class AssistantPanelModel { private readonly listeners = new Set<() => void>(); private active = false; private generation = 0; + private listVersion = 0; private snapshot: AssistantPanel; constructor(private readonly source: AssistantDataSource) { @@ -57,6 +59,7 @@ export class AssistantPanelModel { this.conversations = []; this.listLoading = true; this.listError = undefined; + this.deletionError = undefined; this.selectedKey = 'draft:0'; this.draftKey = 'draft:0'; this.draftSequence = 0; @@ -68,16 +71,20 @@ export class AssistantPanelModel { } private async loadList(generation: number): Promise { + const version = ++this.listVersion; try { const conversations = await this.source.api.listConversations(); - if (!this.active || this.generation !== generation) return; + if (!this.active || this.generation !== generation || this.listVersion !== version) return; const visible = conversations.filter(conversation => !this.deleted.has(conversation.id)); const ids = new Set(visible.map(conversation => conversation.id)); - this.conversations = [...this.conversations.filter(conversation => !ids.has(conversation.id)), ...visible]; + this.conversations = [...this.conversations.filter(conversation => !ids.has(conversation.id)), ...visible].sort( + (a, b) => (Date.parse(b.updatedAt) || 0) - (Date.parse(a.updatedAt) || 0) + ); this.listLoading = false; + this.listError = undefined; } catch (error) { - if (!this.active || this.generation !== generation) return; + if (!this.active || this.generation !== generation || this.listVersion !== version) return; this.listLoading = false; this.listError = error instanceof Error ? error.message : 'Could not load conversations.'; @@ -87,11 +94,22 @@ export class AssistantPanelModel { } private createSession(key: string, conversation?: AssistantConversation): AssistantConversationSession { - const session = new AssistantConversationSession(key, conversation, this.source, this.publish, this.confirmed); + const session = new AssistantConversationSession( + key, + conversation, + this.source, + this.publish, + this.confirmed, + this.refreshConversations + ); this.sessions.set(key, session); return session; } + private readonly refreshConversations = (): void => { + if (this.active) void this.loadList(this.generation); + }; + private readonly confirmed = (session: AssistantConversationSession, conversation: AssistantConversation): void => { if (!this.active || this.sessions.get(session.key) !== session) return; @@ -168,14 +186,14 @@ export class AssistantPanelModel { if (session.snapshot.deleting) return false; const generation = this.generation; - this.listError = undefined; + this.deletionError = undefined; const result = await session.delete(); if (!this.active || this.generation !== generation || this.sessions.get(session.key) !== session) return false; if (result.status === 'ignored') return false; if (result.status === 'failed') { - this.listError = result.message; + this.deletionError = result.message; this.publish(); return false; } @@ -198,7 +216,7 @@ export class AssistantPanelModel { return { conversations: this.conversations, listLoading: this.listLoading, - listError: this.listError, + listError: this.deletionError ?? this.listError, selectedKey: this.selectedKey, entry: this.sessions.get(this.selectedKey)!.snapshot, creatingConversation: !!this.draftKey && this.sessions.get(this.draftKey)!.snapshot.phase === 'creating', diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx index 0b7ef437a4c..0ae87e4fb6c 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx @@ -1501,4 +1501,98 @@ describe('useAssistantPanel', () => { expect(panel.result.current.entry.draft).toBe('Keep this question'); }); + it('refreshes activity metadata and ordering after a send in an existing conversation', async () => { + const source = createAssistantFixture({ + conversations: ASSISTANT_HISTORY_CONVERSATIONS, + now: FIXTURE_NOW, + delayMs: 0 + }); + const panel = await mountPanel(source); + await act(async () => panel.result.current.select('c2')); + await act(async () => panel.result.current.send('Follow-up question')); + + const saved = await source.api.listConversations(); + expect(panel.result.current.conversations).toEqual(saved); + expect(panel.result.current.conversations[0].id).toBe('c2'); + expect(panel.result.current.conversations[0].updatedAt).toBe(new Date(FIXTURE_NOW + 1_000).toISOString()); + }); + + it('orders refreshed activity by instant across ISO timestamp formats', async () => { + const harness = sourceHarness(); + const panel = await mountPanel(harness.source); + const older = { ...conversation('a'), updatedAt: '2026-10-19T12:00:00Z' }; + const newer = { ...conversation('b'), updatedAt: '2026-10-19T12:00:00.001Z' }; + harness.api.listConversations.mockResolvedValueOnce([older, newer]); + await act(async () => panel.result.current.select('a')); + const sending = await sendQuestion(panel, 'Refresh activity'); + await act(async () => { + harness.feeds[0].push({ type: 'run.started' }, { type: 'run.completed' }); + await sending.completion; + }); + + expect(panel.result.current.conversations.map(item => item.id)).toEqual(['b', 'a', 'read-only']); + }); + + it('preserves a deletion failure when an activity refresh succeeds later', async () => { + const harness = sourceHarness(); + const panel = await mountPanel(harness.source); + const refresh = deferred(); + harness.api.listConversations.mockImplementationOnce(() => refresh.promise); + await act(async () => panel.result.current.select('a')); + const sending = await sendQuestion(panel, 'Refresh activity'); + await act(async () => { + harness.feeds[0].push({ type: 'run.started' }, { type: 'run.completed' }); + await sending.completion; + }); + harness.api.deleteConversation.mockRejectedValueOnce(new Error('Conversation busy')); + await act(async () => { + await panel.result.current.deleteConversation('b'); + }); + expect(panel.result.current.listError).toBe('Conversation busy'); + await act(async () => refresh.resolve([conversation('a'), conversation('b')])); + + expect(panel.result.current.listError).toBe('Conversation busy'); + expect(panel.result.current.entry.failure).toBeUndefined(); + }); + + it('clears a failed list read after a successful activity refresh', async () => { + const harness = sourceHarness(); + harness.api.listConversations.mockRejectedValueOnce(new Error('List unavailable')); + const panel = await mountPanel(harness.source); + expect(panel.result.current.listError).toBe('List unavailable'); + const sending = await sendQuestion(panel, 'Start a conversation'); + await act(async () => { + harness.feeds[0].push({ type: 'run.started' }, { type: 'run.completed' }); + await sending.completion; + }); + + expect(panel.result.current.listError).toBeUndefined(); + expect(panel.result.current.conversations.map(item => item.id)).toContain('a'); + }); + + it('ignores an older activity refresh that resolves after a newer send', async () => { + const harness = sourceHarness(); + const older = deferred(); + const newer = deferred(); + const panel = await mountPanel(harness.source); + harness.api.listConversations + .mockImplementationOnce(() => older.promise) + .mockImplementationOnce(() => newer.promise); + await act(async () => panel.result.current.select('a')); + const first = await sendQuestion(panel, 'First'); + await act(async () => { + harness.feeds[0].push({ type: 'run.started' }, { type: 'run.completed' }); + await first.completion; + }); + const second = await sendQuestion(panel, 'Second'); + await act(async () => { + harness.feeds[1].push({ type: 'run.started' }, { type: 'run.completed' }); + await second.completion; + }); + const fresh = { ...conversation('a'), updatedAt: '2026-10-20T00:00:00Z' }; + await act(async () => newer.resolve([fresh])); + await act(async () => older.resolve([conversation('a')])); + + expect(panel.result.current.conversations.find(item => item.id === 'a')).toEqual(fresh); + }); }); From a13d1b4d7ae67a9965305b11c4cb4b104fc1ea54 Mon Sep 17 00:00:00 2001 From: miinhho Date: Thu, 22 Oct 2026 04:31:51 +0900 Subject: [PATCH 07/10] [ZEPPELIN-6748] Fix conversation metadata and deletion error ownership --- .../lib/useDeleteConfirmation.ts | 28 ++- .../model/assistantConversation.ts | 28 ++- .../model/assistantPanelModel.ts | 43 ++--- .../model/assistantPanelState.ts | 5 +- .../model/useAssistantPanel.spec.tsx | 165 +++++++++++++++--- .../ui/AssistantConversationPanel.spec.tsx | 25 +++ .../ui/AssistantConversationPanel.tsx | 1 - 7 files changed, 233 insertions(+), 62 deletions(-) diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/lib/useDeleteConfirmation.ts b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/lib/useDeleteConfirmation.ts index ea1719f155d..929a5c97b11 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/lib/useDeleteConfirmation.ts +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/lib/useDeleteConfirmation.ts @@ -13,14 +13,14 @@ import { useCallback, useLayoutEffect, useRef, useState } from 'react'; import type { AssistantConversation } from '@/entities/assistant/model/assistantContract'; import { conversationTitle } from '@/entities/assistant/ui/conversationTitle'; +import type { AssistantDeletionResult } from '../model/assistantPanelState'; interface DeleteConfirmationOptions { scope: object; conversations: readonly AssistantConversation[]; listLoading: boolean; deletingConversationIds: readonly string[]; - error?: string; - onDelete: (id: string) => Promise; + onDelete: (id: string) => Promise; fallbackFocus: () => HTMLElement | null; } @@ -32,7 +32,11 @@ interface DeleteTarget { type DeleteInteraction = { scope: object; trigger: HTMLElement | null; -} & ({ kind: 'open'; target: DeleteTarget; status: 'ready' | 'pending' | 'failed' } | { kind: 'closed' }); +} & ( + | { kind: 'open'; target: DeleteTarget; status: 'ready' | 'pending' } + | { kind: 'open'; target: DeleteTarget; status: 'failed'; message: string } + | { kind: 'closed' } +); /** Closing the confirmation does not cancel an already requested deletion. */ export const useDeleteConfirmation = ({ @@ -40,7 +44,6 @@ export const useDeleteConfirmation = ({ conversations, listLoading, deletingConversationIds, - error, onDelete, fallbackFocus }: DeleteConfirmationOptions) => { @@ -112,14 +115,21 @@ export const useDeleteConfirmation = ({ ) return; - const pending: DeleteInteraction = { ...active, status: 'pending' }; + const pending: DeleteInteraction = { + kind: 'open', + scope, + trigger: active.trigger, + target: active.target, + status: 'pending' + }; transition(pending); - const removed = await onDelete(active.target.id); + const result = await onDelete(active.target.id); if (current.current !== pending) return; - if (removed) transition({ kind: 'closed', scope, trigger: pending.trigger }); - else transition({ ...pending, status: 'failed' }); + if (result.status === 'deleted') transition({ kind: 'closed', scope, trigger: pending.trigger }); + else if (result.status === 'failed') transition({ ...pending, status: 'failed', message: result.message }); + else transition({ ...pending, status: 'ready' }); }; const active = interaction?.scope === scope && interaction.kind === 'open' ? interaction : null; @@ -127,7 +137,7 @@ export const useDeleteConfirmation = ({ return { target: active?.target ?? null, deleting: !!active && (active.status === 'pending' || deletingConversationIds.includes(active.target.id)), - error: active?.status === 'failed' ? (error ?? 'Could not delete the conversation.') : undefined, + error: active?.status === 'failed' ? active.message : undefined, request, cancel, confirm diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts index f96e626f769..6420113ab80 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts @@ -13,15 +13,17 @@ import { AssistantStreamError, type AssistantConversation } from '@/entities/assistant/model/assistantContract'; import type { AssistantDataSource } from './assistantDataSource'; import { AssistantHistory } from './assistantHistory'; -import type { AssistantPanelEntry, AssistantPanelFailure, AssistantPanelPhase } from './assistantPanelState'; +import type { + AssistantDeletionResult, + AssistantPanelEntry, + AssistantPanelFailure, + AssistantPanelPhase +} from './assistantPanelState'; import { AssistantRun, type AssistantRunSnapshot } from './assistantRun'; const UNKNOWN_OUTCOME = 'The outcome of an earlier question is unknown. Reload can show saved messages, but cannot establish whether that run has finished. Sending again is an explicit new submission.'; -export type AssistantDeletionResult = - { status: 'deleted' } | { status: 'ignored' } | { status: 'failed'; message: string }; - export class AssistantConversationSession { private conversation?: AssistantConversation; private draft = ''; @@ -54,6 +56,24 @@ export class AssistantConversationSession { return this.entry; } + /** Does not emit a change notification. */ + updateConversation(conversation: AssistantConversation): void { + const current = this.conversation; + if (this.disposed || current?.id !== conversation.id) return; + + if ( + current.title === conversation.title && + current.ownerId === conversation.ownerId && + current.canSendMessage === conversation.canSendMessage && + current.updatedAt === conversation.updatedAt + ) { + return; + } + + this.conversation = conversation; + this.entry = this.project(); + } + private project(): AssistantPanelEntry { return { ...this.history.snapshot, diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantPanelModel.ts b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantPanelModel.ts index 3f1ce5455f4..af562c431ce 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantPanelModel.ts +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantPanelModel.ts @@ -13,13 +13,12 @@ import type { AssistantConversation } from '@/entities/assistant/model/assistantContract'; import { AssistantConversationSession } from './assistantConversation'; import type { AssistantDataSource } from './assistantDataSource'; -import type { AssistantPanel } from './assistantPanelState'; +import type { AssistantDeletionResult, AssistantPanel } from './assistantPanelState'; export class AssistantPanelModel { private conversations: AssistantConversation[] = []; private listLoading = true; private listError?: string; - private deletionError?: string; private selectedKey = 'draft:0'; private draftKey: string | undefined = 'draft:0'; @@ -59,7 +58,6 @@ export class AssistantPanelModel { this.conversations = []; this.listLoading = true; this.listError = undefined; - this.deletionError = undefined; this.selectedKey = 'draft:0'; this.draftKey = 'draft:0'; this.draftSequence = 0; @@ -77,10 +75,18 @@ export class AssistantPanelModel { if (!this.active || this.generation !== generation || this.listVersion !== version) return; const visible = conversations.filter(conversation => !this.deleted.has(conversation.id)); - const ids = new Set(visible.map(conversation => conversation.id)); - this.conversations = [...this.conversations.filter(conversation => !ids.has(conversation.id)), ...visible].sort( - (a, b) => (Date.parse(b.updatedAt) || 0) - (Date.parse(a.updatedAt) || 0) - ); + const refreshed = new Map(visible.map(conversation => [conversation.id, conversation])); + this.conversations = [ + ...this.conversations.filter(conversation => !refreshed.has(conversation.id)), + ...visible + ].sort((a, b) => (Date.parse(b.updatedAt) || 0) - (Date.parse(a.updatedAt) || 0)); + + this.sessions.forEach(session => { + const id = session.snapshot.conversation?.id; + const conversation = id && refreshed.get(id); + if (conversation) session.updateConversation(conversation); + }); + this.listLoading = false; this.listError = undefined; } catch (error) { @@ -174,36 +180,31 @@ export class AssistantPanelModel { return this.sessions.get(this.selectedKey)!.loadEarlier(); }; - readonly deleteConversation = async (id: string): Promise => { - if (!this.active) return false; + readonly deleteConversation = async (id: string): Promise => { + if (!this.active) return { status: 'ignored' }; const conversation = this.conversations.find(item => item.id === id); - if (!conversation) return false; + if (!conversation) return { status: 'ignored' }; const session = Array.from(this.sessions.values()).find(item => item.snapshot.conversation?.id === id) ?? this.createSession(`conversation:${id}`, conversation); - if (session.snapshot.deleting) return false; + if (session.snapshot.deleting) return { status: 'ignored' }; const generation = this.generation; - this.deletionError = undefined; const result = await session.delete(); - if (!this.active || this.generation !== generation || this.sessions.get(session.key) !== session) return false; - if (result.status === 'ignored') return false; - - if (result.status === 'failed') { - this.deletionError = result.message; - this.publish(); - return false; + if (!this.active || this.generation !== generation || this.sessions.get(session.key) !== session) { + return { status: 'ignored' }; } + if (result.status !== 'deleted') return result; this.deleted.add(id); this.sessions.delete(session.key); this.conversations = this.conversations.filter(item => item.id !== id); if (this.selectedKey === session.key) this.selectDraft(); this.publish(); - return true; + return result; }; private project(): AssistantPanel { @@ -216,7 +217,7 @@ export class AssistantPanelModel { return { conversations: this.conversations, listLoading: this.listLoading, - listError: this.deletionError ?? this.listError, + listError: this.listError, selectedKey: this.selectedKey, entry: this.sessions.get(this.selectedKey)!.snapshot, creatingConversation: !!this.draftKey && this.sessions.get(this.draftKey)!.snapshot.phase === 'creating', diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantPanelState.ts b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantPanelState.ts index 23c4e083490..9fe377fe939 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantPanelState.ts +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantPanelState.ts @@ -22,6 +22,9 @@ export interface AssistantPanelFailure { recovery?: AssistantRecovery; } +export type AssistantDeletionResult = + { status: 'deleted' } | { status: 'ignored' } | { status: 'failed'; message: string }; + export interface AssistantPanelHistory { messages: AssistantMessage[]; /** Message IDs mapped to stable rendered-row keys. */ @@ -78,7 +81,7 @@ export interface AssistantPanelActions { /** Resolves to true only when an earlier page is applied to the current history. */ loadEarlier(): Promise; - deleteConversation(id: string): Promise; + deleteConversation(id: string): Promise; } export interface AssistantPanel extends AssistantPanelSnapshot, AssistantPanelActions {} diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx index 0ae87e4fb6c..eb9f260006a 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx @@ -24,7 +24,7 @@ import type { AssistantDataSource } from './assistantDataSource'; import { createAssistantFixture } from '../fixtures/assistantFixture'; import { ASSISTANT_HISTORY_CONVERSATIONS, FIXTURE_NOW } from '../fixtures/assistantFixtureData'; import { AssistantPanelModel } from './assistantPanelModel'; -import type { AssistantPanel } from './assistantPanelState'; +import type { AssistantDeletionResult, AssistantPanel } from './assistantPanelState'; import { useAssistantPanel } from './useAssistantPanel'; const conversation = (id: string, canSendMessage = true): AssistantConversation => ({ @@ -246,7 +246,7 @@ describe('useAssistantPanel', () => { await act(async () => { const deleted = await panel.result.current.deleteConversation('a'); - expect(deleted).toBe(true); + expect(deleted).toEqual({ status: 'deleted' }); }); expect(panel.result.current.entry).toMatchObject({ key: draftKey, phase: 'creating', draft: 'Still creating' }); @@ -265,7 +265,7 @@ describe('useAssistantPanel', () => { await act(async () => { const deleted = await panel.result.current.deleteConversation('b'); - expect(deleted).toBe(true); + expect(deleted).toEqual({ status: 'deleted' }); }); expect(panel.result.current.selectedKey).toBe(draftKey); @@ -287,7 +287,7 @@ describe('useAssistantPanel', () => { await act(async () => { const deleted = await panel.result.current.deleteConversation('created'); - expect(deleted).toBe(true); + expect(deleted).toEqual({ status: 'deleted' }); }); const fallbackKey = panel.result.current.selectedKey; @@ -1011,22 +1011,22 @@ describe('useAssistantPanel', () => { const panel = await mountPanel(harness.source); act(() => panel.result.current.select('a')); - let deleted = true; + let deletion!: AssistantDeletionResult; await act(async () => { - deleted = await panel.result.current.deleteConversation('a'); + deletion = await panel.result.current.deleteConversation('a'); }); - expect(deleted).toBe(false); + expect(deletion).toEqual({ status: 'failed', message: 'Conversation busy' }); expect(panel.result.current.conversations.map(item => item.id)).toContain('a'); expect(panel.result.current.entry.failure).toEqual({ message: 'Conversation busy', kind: 'failed' }); - expect(panel.result.current.listError).toBe('Conversation busy'); + expect(panel.result.current.listError).toBeUndefined(); expect(panel.result.current.entry.failure?.recovery).toBeUndefined(); await act(async () => { - deleted = await panel.result.current.deleteConversation('a'); + deletion = await panel.result.current.deleteConversation('a'); }); - expect(deleted).toBe(true); + expect(deletion).toEqual({ status: 'deleted' }); expect(panel.result.current.conversations.map(item => item.id)).not.toContain('a'); await act(async () => @@ -1045,7 +1045,7 @@ describe('useAssistantPanel', () => { const panel = await mountPanel(harness.source); await act(async () => panel.result.current.select('a')); - let removing!: Promise; + let removing!: Promise; act(() => { removing = panel.result.current.deleteConversation('a'); }); @@ -1089,12 +1089,12 @@ describe('useAssistantPanel', () => { expect(panel.result.current.entry.phase).toBe('unknown'); - let deleted = false; + let deletion!: AssistantDeletionResult; await act(async () => { - deleted = await panel.result.current.deleteConversation('a'); + deletion = await panel.result.current.deleteConversation('a'); }); - expect(deleted).toBe(true); + expect(deletion).toEqual({ status: 'deleted' }); expect(harness.api.deleteConversation).toHaveBeenCalledWith('a'); expect(panel.result.current.conversations.map(item => item.id)).not.toContain('a'); expect(panel.result.current.entry.conversation).toBeUndefined(); @@ -1116,7 +1116,7 @@ describe('useAssistantPanel', () => { ); const signal = harness.run.mock.calls[0][2]; - let removing!: Promise; + let removing!: Promise; act(() => { removing = panel.result.current.deleteConversation('a'); }); @@ -1131,13 +1131,13 @@ describe('useAssistantPanel', () => { expect(panel.result.current.entry.answer).toMatchObject({ content: 'Before deletion while deleting' }); - let deleted = true; + let result!: AssistantDeletionResult; await act(async () => { deletion.reject(new Error('409: Conversation busy')); - deleted = await removing; + result = await removing; }); - expect(deleted).toBe(false); + expect(result).toEqual({ status: 'failed', message: '409: Conversation busy' }); expect(signal.aborted).toBe(false); expect(panel.result.current.entry).toMatchObject({ phase: 'running', @@ -1172,7 +1172,7 @@ describe('useAssistantPanel', () => { await act(async () => harness.feeds[0].push({ type: 'run.started' })); const signal = harness.run.mock.calls[0][2]; - let removing!: Promise; + let removing!: Promise; act(() => { removing = panel.result.current.deleteConversation('a'); }); @@ -1180,13 +1180,13 @@ describe('useAssistantPanel', () => { expect(signal.aborted).toBe(false); expect(panel.result.current.entry.conversation?.id).toBe('a'); - let deleted = false; + let result!: AssistantDeletionResult; await act(async () => { deletion.resolve(undefined); - deleted = await removing; + result = await removing; }); - expect(deleted).toBe(true); + expect(result).toEqual({ status: 'deleted' }); expect(signal.aborted).toBe(true); expect(panel.result.current.conversations.map(item => item.id)).not.toContain('a'); @@ -1533,9 +1533,10 @@ describe('useAssistantPanel', () => { expect(panel.result.current.conversations.map(item => item.id)).toEqual(['b', 'a', 'read-only']); }); - it('preserves a deletion failure when an activity refresh succeeds later', async () => { + it('preserves a session deletion failure across activity refresh without changing list errors', async () => { const harness = sourceHarness(); const panel = await mountPanel(harness.source); + await act(async () => panel.result.current.select('b')); const refresh = deferred(); harness.api.listConversations.mockImplementationOnce(() => refresh.promise); await act(async () => panel.result.current.select('a')); @@ -1545,14 +1546,23 @@ describe('useAssistantPanel', () => { await sending.completion; }); harness.api.deleteConversation.mockRejectedValueOnce(new Error('Conversation busy')); + let result!: AssistantDeletionResult; + await act(async () => { - await panel.result.current.deleteConversation('b'); + result = await panel.result.current.deleteConversation('b'); }); - expect(panel.result.current.listError).toBe('Conversation busy'); + + expect(result).toEqual({ status: 'failed', message: 'Conversation busy' }); + expect(panel.result.current.listError).toBeUndefined(); + await act(async () => refresh.resolve([conversation('a'), conversation('b')])); - expect(panel.result.current.listError).toBe('Conversation busy'); + expect(panel.result.current.listError).toBeUndefined(); expect(panel.result.current.entry.failure).toBeUndefined(); + + await act(async () => panel.result.current.select('b')); + + expect(panel.result.current.entry.failure).toEqual({ message: 'Conversation busy', kind: 'failed' }); }); it('clears a failed list read after a successful activity refresh', async () => { @@ -1595,4 +1605,107 @@ describe('useAssistantPanel', () => { expect(panel.result.current.conversations.find(item => item.id === 'a')).toEqual(fresh); }); + + it.each([true, false])( + 'applies refreshed permissions to an existing session that was writable=%s', + async initiallyWritable => { + const harness = sourceHarness(); + harness.api.listConversations.mockResolvedValue([conversation('a', initiallyWritable), conversation('b')]); + const saved: AssistantMessagePage = { + messages: [{ id: 'saved-question', role: 'user', content: 'Keep this history' }], + cursor: 'older' + }; + harness.api.getMessages.mockResolvedValue(saved); + const panel = await mountPanel(harness.source); + await act(async () => panel.result.current.select('a')); + act(() => panel.result.current.setDraft('Keep the next A question')); + const key = panel.result.current.entry.key; + const refreshed = { + ...conversation('a', !initiallyWritable), + title: 'Updated A', + ownerId: 'updated-owner', + updatedAt: '2026-10-20T00:00:00Z' + }; + harness.api.listConversations.mockResolvedValue([refreshed, conversation('b')]); + await act(async () => panel.result.current.select('b')); + + const sending = await sendQuestion(panel, 'Refresh from B'); + await act(async () => { + harness.feeds[0].push({ type: 'run.started' }, { type: 'run.completed' }); + await sending.completion; + }); + await act(async () => panel.result.current.select('a')); + + expect(panel.result.current.entry).toMatchObject({ + key, + conversation: refreshed, + draft: 'Keep the next A question', + messages: saved.messages, + cursor: 'older' + }); + + const followUp = await sendQuestion(panel); + + expect(harness.run.mock.calls.map(([id, content]) => [id, content])).toEqual( + initiallyWritable + ? [['b', 'Refresh from B']] + : [ + ['b', 'Refresh from B'], + ['a', 'Keep the next A question'] + ] + ); + expect(panel.result.current.entry.draft).toBe('Keep the next A question'); + expect(panel.result.current.entry.phase).toBe(initiallyWritable ? 'idle' : 'pending'); + + await act(async () => { + if (!initiallyWritable) harness.feeds[1].push({ type: 'run.failed', message: 'Stop follow-up' }); + await followUp.completion; + }); + } + ); + + it('updates metadata without interrupting an active answer or clearing its next draft', async () => { + const harness = sourceHarness(); + const panel = await mountPanel(harness.source); + await act(async () => panel.result.current.select('a')); + const first = await sendQuestion(panel, 'Question for A'); + await act(async () => + harness.feeds[0].push( + { type: 'run.started' }, + { type: 'message.delta', messageId: 'answer-a', delta: 'A is running' } + ) + ); + act(() => panel.result.current.setDraft('Next A question')); + const key = panel.result.current.entry.key; + const refreshed = { ...conversation('a', false), title: 'Updated running A' }; + harness.api.listConversations.mockResolvedValue([refreshed, conversation('b')]); + await act(async () => panel.result.current.select('b')); + + const second = await sendQuestion(panel, 'Question for B'); + await act(async () => { + harness.feeds[1].push({ type: 'run.started' }, { type: 'run.completed' }); + await second.completion; + }); + await act(async () => panel.result.current.select('a')); + + expect(panel.result.current.entry).toMatchObject({ + key, + conversation: refreshed, + phase: 'running', + draft: 'Next A question', + answer: { status: 'streaming', content: 'A is running' } + }); + expect(harness.run.mock.calls[0][2].aborted).toBe(false); + + await act(async () => { + harness.feeds[0].push( + { type: 'message.delta', messageId: 'answer-a', delta: ' and finishes' }, + { type: 'run.failed', message: 'Stop A' } + ); + await first.completion; + }); + + expect(panel.result.current.entry.answer).toMatchObject({ content: 'A is running and finishes' }); + expect(panel.result.current.entry.draft).toBe('Next A question'); + }); }); diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.spec.tsx index d72d4878f87..633a4322d09 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.spec.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.spec.tsx @@ -833,4 +833,29 @@ describe('AssistantConversationPanel', () => { expect(screen.getByText('The next question was rejected.')).toBeTruthy(); } ); + + it('keeps B deletion failure in its dialog when a dismissed A deletion fails later', async () => { + const source = fixture(); + const a = Promise.withResolvers(); + const b = Promise.withResolvers(); + vi.spyOn(source.api, 'deleteConversation').mockImplementation(id => (id === 'c1' ? a.promise : b.promise)); + await mountPanel(source); + fireEvent.click(listTrigger()); + fireEvent.click(screen.getByRole('button', { name: 'Delete Revenue analysis' })); + fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Delete conversation' })); + fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Cancel' })); + fireEvent.click(screen.getByRole('button', { name: 'Delete Notebook summary' })); + fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Delete conversation' })); + + await act(async () => b.reject(new Error('B deletion denied'))); + + expect(within(screen.getByRole('dialog')).getByRole('alert').textContent).toContain('B deletion denied'); + + await act(async () => a.reject(new Error('A is still running'))); + + const dialog = screen.getByRole('dialog'); + expect(within(dialog).getByText(/Delete “Notebook summary”/)).toBeTruthy(); + expect(within(dialog).getByRole('alert').textContent).toContain('B deletion denied'); + expect(within(dialog).queryByText('A is still running')).toBeNull(); + }); }); diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.tsx index 155a82cdd61..e163bbdb9f6 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.tsx @@ -44,7 +44,6 @@ export const AssistantConversationPanel = ({ source, now }: AssistantConversatio conversations: panel.conversations, listLoading: panel.listLoading, deletingConversationIds: panel.deletingConversationIds, - error: panel.listError, onDelete: panel.deleteConversation, fallbackFocus }); From 142450836ca9e28cbabfd3bb890dc8b364aac06e Mon Sep 17 00:00:00 2001 From: miinhho Date: Fri, 23 Oct 2026 03:05:34 +0900 Subject: [PATCH 08/10] [ZEPPELIN-6748] Restore conversation history after run and deletion failures --- .../model/assistantConversation.ts | 20 +- .../model/assistantHistory.ts | 68 ++++- .../model/useAssistantPanel.spec.tsx | 273 ++++++++++++++++++ 3 files changed, 349 insertions(+), 12 deletions(-) diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts index 6420113ab80..94b5187adf3 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts @@ -32,6 +32,7 @@ export class AssistantConversationSession { private uncertainty?: string; private submittedQuestion?: string; private deleting = false; + private historyRefreshPending = false; private disposed = false; private controller?: AbortController; @@ -112,6 +113,7 @@ export class AssistantConversationSession { return; } + this.historyRefreshPending = false; if (!preserveFailure) this.failure = undefined; const result = await this.history.loadLatest(this.source.api, this.conversation.id); if (this.disposed || result.status === 'ignored' || !this.history.isCurrent(result)) return; @@ -122,13 +124,19 @@ export class AssistantConversationSession { kind: 'connection', recovery: 'reload' }; - } else if (this.phase !== 'unknown' && !preserveFailure) { + } else if (this.phase !== 'unknown' && !preserveFailure && this.history.snapshot.answerRecovery !== 'send') { this.submittedQuestion = undefined; } this.publish(); } + private async refreshHistory(): Promise { + if (!this.historyRefreshPending || this.disposed || this.deleting || this.phase !== 'idle') return; + + await this.reload(this.failure?.kind === 'failed'); + } + async loadEarlier(): Promise { if (this.disposed || !this.conversation || this.deleting || (this.phase !== 'idle' && this.phase !== 'unknown')) { return false; @@ -173,6 +181,7 @@ export class AssistantConversationSession { this.submittedQuestion = content; this.failure = undefined; this.phase = this.conversation ? 'pending' : 'creating'; + if (this.history.snapshot.loading) this.historyRefreshPending = true; const answerKey = this.history.beginTurn(); if (!this.conversation) { @@ -246,15 +255,13 @@ export class AssistantConversationSession { if (this.disposed) return; // The next send must wait for this consumer to release the conversation. + this.historyRefreshPending ||= + run.snapshot.outcome === 'completed' || (run.snapshot.outcome === 'failed' && !this.history.snapshot.loaded); this.phase = run.snapshot.outcome === 'unknown' ? 'unknown' : 'idle'; this.publish(); if (run.snapshot.accepted) this.activity(); - if (run.snapshot.outcome === 'completed') await this.reload(); - else if (!run.snapshot.accepted && !this.history.snapshot.loaded && run.snapshot.outcome === 'failed') { - // A rejected send invalidated the first history read; restore it without losing the retryable question. - await this.reload(true); - } + await this.refreshHistory(); } async delete(): Promise { @@ -276,6 +283,7 @@ export class AssistantConversationSession { this.deleting = false; this.failure = { message, kind: 'failed' }; this.publish(); + void this.refreshHistory(); return { status: 'failed', message }; } diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantHistory.ts b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantHistory.ts index 74693dee3ee..37769baa08a 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantHistory.ts +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantHistory.ts @@ -131,6 +131,44 @@ interface RetainedTurn { recovery?: AssistantRecovery; } +/** Restores saved history without replacing the current failed turn's runtime error and partial answer. */ +const retainFailedTurn = ( + entry: AssistantPanelHistory, + messages: AssistantMessage[], + keys: Map, + turn?: RetainedTurn +): Pick => { + if (turn?.recovery !== 'send') return { messages, messageKeys: keys }; + + const question = entry.messages.find( + message => message.role === 'user' && (entry.messageKeys.get(message.id) ?? message.id) === turn.questionKey + ); + if (!question || question.role !== 'user') return { messages, messageKeys: keys }; + + const messageKeys = new Map(keys); + let savedQuestion = + messages.find( + message => + message.role === 'user' && message.id !== question.id && messageKeys.get(message.id) === turn.questionKey + ) ?? messages.find(message => message.id === question.id); + + // The fixture has one writer: only this failed turn's trailing question may match without a shared user ID. + const trailing = messages.at(-1); + if (!savedQuestion && trailing?.role === 'user' && trailing.content === question.content) savedQuestion = trailing; + if (savedQuestion) messageKeys.set(savedQuestion.id, turn.questionKey); + + const retained = messages.filter( + message => + message.role !== 'user' || + !savedQuestion || + message.id === savedQuestion.id || + (messageKeys.get(message.id) ?? message.id) !== turn.questionKey + ); + if (!savedQuestion) retained.push(question); + + return { messages: retained, messageKeys }; +}; + export class AssistantHistory { private messages: AssistantMessage[] = []; private messageKeys = new Map(); @@ -156,8 +194,15 @@ export class AssistantHistory { } get snapshot(): AssistantPanelHistory { + const failedKey = this.turn?.recovery === 'send' ? this.turn.key : undefined; + return { - messages: this.messages, + // Keep saved answer boundaries for pagination, but render the retained failed answer only once. + messages: failedKey + ? this.messages.filter( + message => message.role !== 'assistant' || this.messageKeys.get(message.id) !== failedKey + ) + : this.messages, messageKeys: this.messageKeys, localAnswerViews: this.localAnswerViews, cursor: this.cursor, @@ -189,7 +234,13 @@ export class AssistantHistory { if (!this.turn?.message) return; const { message, key, view } = this.turn; - this.messages = [...this.messages, message]; + const saved = this.messages.findIndex( + current => current.role === 'assistant' && this.messageKeys.get(current.id) === key + ); + this.messages = + saved < 0 + ? [...this.messages, message] + : this.messages.map((current, index) => (index === saved ? message : current)); this.messageKeys = new Map(this.messageKeys).set(message.id, key); if (view) this.localAnswerViews = new Map(this.localAnswerViews).set(message.id, view); } @@ -230,11 +281,12 @@ export class AssistantHistory { if (!this.current(version)) return { status: 'ignored' }; const reconciled = reconcileLatestHistory(this.snapshot, page, this.turn?.message); - this.messages = reconciled.messages; - this.messageKeys = reconciled.messageKeys; + const retained = retainFailedTurn(this.snapshot, reconciled.messages, reconciled.messageKeys, this.turn); + this.messages = retained.messages; + this.messageKeys = retained.messageKeys; this.localAnswerViews = reconciled.localAnswerViews; this.cursor = reconciled.cursor; - this.turn = undefined; + if (this.turn?.recovery !== 'send') this.turn = undefined; this.loaded = true; this.requestStatus = 'idle'; this.consumedCursors.clear(); @@ -266,7 +318,11 @@ export class AssistantHistory { const page = await api.getMessages(conversationId, cursor); if (!this.current(version)) return { status: 'ignored' }; - this.messages = joinPages(page.messages, this.messages); + const messages = joinPages(page.messages, this.messages); + const keys = reconcileMessageKeys(this.snapshot, messages, this.turn?.message); + const retained = retainFailedTurn(this.snapshot, messages, keys, this.turn); + this.messages = retained.messages; + this.messageKeys = retained.messageKeys; this.cursor = page.cursor; this.requestStatus = 'idle'; this.consumedCursors.add(cursor); diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx index eb9f260006a..0f7642d0a02 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx @@ -1708,4 +1708,277 @@ describe('useAssistantPanel', () => { expect(panel.result.current.entry.answer).toMatchObject({ content: 'A is running and finishes' }); expect(panel.result.current.entry.draft).toBe('Next A question'); }); + + it.each([false, true])( + 'recovers interrupted initial history without losing a failed answer, persisted partial=%s', + async persisted => { + const harness = sourceHarness(); + const initial = Promise.withResolvers(); + const content = 'Repeated question'; + const questions: AssistantMessagePage['messages'] = [ + { id: 'older-question', role: 'user', content }, + { id: 'older-answer', role: 'assistant', segments: [{ id: 'older-round', content: 'Older answer' }] }, + { id: 'saved-question', role: 'user', content } + ]; + harness.api.getMessages + .mockImplementationOnce(() => initial.promise) + .mockResolvedValueOnce({ + messages: persisted + ? [...questions, { id: 'saved-answer', role: 'assistant', segments: [{ id: 'round', content: 'Partial' }] }] + : questions, + cursor: 'earlier' + }); + const panel = await mountPanel(harness.source); + act(() => panel.result.current.select('a')); + const sending = await sendQuestion(panel, content); + await act(async () => + harness.feeds[0].push( + { type: 'run.started' }, + { type: 'message.done', messageId: 'round', content: 'Partial' }, + { type: 'tool_call.started', toolCallId: 'tool', name: 'read_note' }, + { type: 'tool_call.done', toolCallId: 'tool' }, + { type: 'message.delta', messageId: 'continuation', delta: 'Unsaved continuation' } + ) + ); + const answerKey = panel.result.current.entry.answerKey; + const questionKey = panel.result.current.entry.messages.at(-1)!.id; + + await act(async () => { + harness.feeds[0].push({ type: 'run.failed', message: 'Execution failed' }); + await sending.completion; + initial.resolve({ messages: [], cursor: 'obsolete' }); + }); + + expect(panel.result.current.entry).toMatchObject({ + loaded: true, + loading: false, + cursor: 'earlier', + draft: content, + answerKey, + answerRecovery: 'send', + answer: { + status: 'error', + message: 'Execution failed', + content: 'Partial\n\nUnsaved continuation', + tools: [{ id: 'tool', name: 'read_note', status: 'done' }] + } + }); + expect(panel.result.current.entry.messages).toEqual(questions); + expect(panel.result.current.entry.messageKeys.get('saved-question')).toBe(questionKey); + expect(harness.run).toHaveBeenCalledTimes(1); + } + ); + + it.each([false, true])( + 'resumes deferred completion history after deletion fails, initial history loaded=%s', + async loaded => { + const harness = sourceHarness(); + const initial = Promise.withResolvers(); + const canonical = Promise.withResolvers(); + const deletion = Promise.withResolvers(); + const saved: AssistantMessagePage = { + messages: [{ id: 'earlier-question', role: 'user', content: 'Earlier question' }], + cursor: 'earlier' + }; + harness.api.getMessages + .mockImplementationOnce(() => (loaded ? Promise.resolve(saved) : initial.promise)) + .mockImplementationOnce(() => canonical.promise); + harness.api.deleteConversation.mockImplementationOnce(() => deletion.promise); + const panel = await mountPanel(harness.source); + await act(async () => panel.result.current.select('a')); + const sending = await sendQuestion(panel, 'Complete this question'); + await act(async () => + harness.feeds[0].push( + { type: 'run.started' }, + { type: 'message.done', messageId: 'round', content: 'Completed answer' } + ) + ); + let removing!: Promise; + act(() => { + removing = panel.result.current.deleteConversation('a'); + }); + + await act(async () => { + harness.feeds[0].push({ type: 'run.completed' }); + await sending.completion; + }); + + expect(harness.api.getMessages).toHaveBeenCalledTimes(1); + expect(panel.result.current.entry).toMatchObject({ deleting: true, answer: { status: 'done' } }); + + let result!: AssistantDeletionResult; + await act(async () => { + deletion.reject(new Error('Deletion denied')); + result = await removing; + }); + + expect(result).toEqual({ status: 'failed', message: 'Deletion denied' }); + expect(panel.result.current.entry).toMatchObject({ + deleting: false, + loading: true, + failure: { message: 'Deletion denied', kind: 'failed' } + }); + const page: AssistantMessagePage = { + messages: [ + ...saved.messages, + { id: 'saved-question', role: 'user', content: 'Complete this question' }, + { id: 'saved-answer', role: 'assistant', segments: [{ id: 'round', content: 'Completed answer' }] } + ], + cursor: 'earlier' + }; + + await act(async () => { + canonical.resolve(page); + initial.resolve({ messages: [], cursor: 'obsolete' }); + }); + + expect(panel.result.current.entry).toMatchObject({ + loaded: true, + loading: false, + messages: page.messages, + cursor: 'earlier', + failure: { message: 'Deletion denied', kind: 'failed' } + }); + expect(panel.result.current.entry.answer).toBeUndefined(); + expect(harness.api.getMessages).toHaveBeenCalledTimes(2); + expect(harness.run).toHaveBeenCalledTimes(1); + } + ); + + it.each([false, true])( + 'joins the retained failed turn when the earlier page includes a partial=%s', + async includePartial => { + const harness = sourceHarness(); + const initial = Promise.withResolvers(); + harness.api.getMessages + .mockImplementationOnce(() => initial.promise) + .mockResolvedValueOnce({ + messages: [{ id: 'saved-answer', role: 'assistant', segments: [{ id: 'round', content: 'Partial' }] }], + cursor: 'earlier' + }) + .mockResolvedValueOnce({ + messages: [ + { id: 'older-question', role: 'user', content: 'Earlier question' }, + { id: 'saved-question', role: 'user', content: 'Failed question' }, + ...(includePartial + ? [{ id: 'saved-prefix', role: 'assistant' as const, segments: [{ id: 'round', content: 'Partial' }] }] + : []) + ], + cursor: null + }); + const panel = await mountPanel(harness.source); + act(() => panel.result.current.select('a')); + const sending = await sendQuestion(panel, 'Failed question'); + await act(async () => { + harness.feeds[0].push( + { type: 'run.started' }, + { type: 'message.delta', messageId: 'round', delta: 'Partial' }, + { type: 'run.failed', message: 'Execution failed' } + ); + await sending.completion; + }); + const answerKey = panel.result.current.entry.answerKey; + const questionKey = panel.result.current.entry.messages.at(-1)!.id; + + await act(async () => { + await panel.result.current.loadEarlier(); + initial.resolve({ messages: [], cursor: 'obsolete' }); + }); + + expect(panel.result.current.entry.messages).toEqual([ + { id: 'older-question', role: 'user', content: 'Earlier question' }, + { id: 'saved-question', role: 'user', content: 'Failed question' } + ]); + expect(panel.result.current.entry.messageKeys.get('saved-question')).toBe(questionKey); + expect(panel.result.current.entry).toMatchObject({ + cursor: null, + answerKey, + answerRecovery: 'send', + answer: { status: 'error', message: 'Execution failed', content: 'Partial' } + }); + + const retry = await sendQuestion(panel, 'Another question'); + await act(async () => { + harness.feeds[1].push({ type: 'run.failed', message: 'Not accepted' }); + await retry.completion; + }); + + expect(panel.result.current.entry.messages.filter(message => message.role === 'assistant')).toMatchObject([ + { id: 'round', role: 'assistant', segments: [{ id: 'round', content: 'Partial' }] } + ]); + expect(panel.result.current.entry.localAnswerViews.get('round')).toMatchObject({ + status: 'error', + message: 'Execution failed' + }); + } + ); + + it('keeps failed recovery available for explicit reload without retrying automatically', async () => { + const harness = sourceHarness(); + const initial = Promise.withResolvers(); + harness.api.getMessages + .mockImplementationOnce(() => initial.promise) + .mockRejectedValueOnce(new Error('History unavailable')); + const panel = await mountPanel(harness.source); + act(() => panel.result.current.select('a')); + const sending = await sendQuestion(panel, 'Failed question'); + + await act(async () => { + harness.feeds[0].push({ type: 'run.started' }, { type: 'run.failed', message: 'Execution failed' }); + await sending.completion; + initial.resolve({ messages: [], cursor: 'obsolete' }); + }); + + expect(panel.result.current.entry).toMatchObject({ + loaded: false, + failure: { message: 'History unavailable', recovery: 'reload' }, + answer: { status: 'error', message: 'Execution failed' } + }); + expect(harness.api.getMessages).toHaveBeenCalledTimes(2); + harness.api.getMessages.mockResolvedValueOnce({ + messages: [{ id: 'saved-question', role: 'user', content: 'Failed question' }], + cursor: 'earlier' + }); + + await act(async () => panel.result.current.reload()); + + expect(panel.result.current.entry).toMatchObject({ + loaded: true, + cursor: 'earlier', + draft: 'Failed question', + answerRecovery: 'send', + answer: { status: 'error', message: 'Execution failed' } + }); + expect(panel.result.current.entry.messages).toEqual([ + { id: 'saved-question', role: 'user', content: 'Failed question' } + ]); + expect(panel.result.current.entry.failure).toBeUndefined(); + expect(harness.api.getMessages).toHaveBeenCalledTimes(3); + expect(harness.run).toHaveBeenCalledTimes(1); + }); + + it('does not resume deferred history when deletion succeeds after completion', async () => { + const harness = sourceHarness(); + const deletion = Promise.withResolvers(); + harness.api.deleteConversation.mockImplementationOnce(() => deletion.promise); + const panel = await mountPanel(harness.source); + await act(async () => panel.result.current.select('a')); + const sending = await sendQuestion(panel, 'Complete while deleting'); + await act(async () => harness.feeds[0].push({ type: 'run.started' })); + let removing!: Promise; + act(() => { + removing = panel.result.current.deleteConversation('a'); + }); + + await act(async () => { + harness.feeds[0].push({ type: 'run.completed' }); + await sending.completion; + deletion.resolve(); + await removing; + }); + + expect(panel.result.current.conversations.map(item => item.id)).not.toContain('a'); + expect(panel.result.current.entry.conversation).toBeUndefined(); + expect(harness.api.getMessages).toHaveBeenCalledTimes(1); + }); }); From ad85a92e65370080aafe816b7e2897d2377f8e05 Mon Sep 17 00:00:00 2001 From: miinhho Date: Fri, 23 Oct 2026 03:25:40 +0900 Subject: [PATCH 09/10] [ZEPPELIN-6748] Avoid message rendering work during draft edits --- .../model/assistantConversation.ts | 66 ++++++++++--------- .../model/useAssistantPanel.spec.tsx | 20 ++++++ .../ui/AssistantConversationPanel.spec.tsx | 44 ++++++++++++- .../ui/AssistantConversationPanel.tsx | 18 ++--- .../ui/ConversationMessages.tsx | 62 +++++++++-------- 5 files changed, 137 insertions(+), 73 deletions(-) diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts index 94b5187adf3..c6b5781b0fe 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts @@ -96,11 +96,13 @@ export class AssistantConversationSession { this.changed(); }; + /** Draft-only edits preserve the current message snapshot. */ setDraft(value: string): void { - if (this.disposed) return; + if (this.disposed || this.draft === value) return; this.draft = value; - this.publish(); + this.entry = { ...this.entry, draft: value }; + this.changed(); } ensureHistory(): void { @@ -184,33 +186,11 @@ export class AssistantConversationSession { if (this.history.snapshot.loading) this.historyRefreshPending = true; const answerKey = this.history.beginTurn(); - if (!this.conversation) { - try { - const conversation = await this.source.api.createConversation({}); - if (this.disposed || this.controller !== controller) return; - - // A rejected first send must reuse the conversation already created. - this.conversation = conversation; - this.phase = 'pending'; - this.entry = this.project(); - this.confirmed(this, conversation); - } catch (error) { - if (!this.disposed && this.controller === controller) { - this.controller = undefined; - this.phase = 'idle'; - this.failure = { - message: error instanceof Error ? error.message : 'Could not create a conversation.', - kind: 'failed', - recovery: 'send' - }; - this.publish(); - } - return; - } - } + const conversation = this.conversation ?? (await this.createConversation(controller)); + if (!conversation || this.disposed || this.controller !== controller) return; const run = new AssistantRun(answerKey); - const publishRun = (previous: AssistantRunSnapshot, next: AssistantRunSnapshot) => { + const applyRunTransition = (previous: AssistantRunSnapshot, next: AssistantRunSnapshot) => { if (!previous.accepted && next.accepted) { this.phase = 'running'; if (this.draft === submitted) this.draft = ''; @@ -231,12 +211,12 @@ export class AssistantConversationSession { }; try { - for await (const event of this.source.run(this.conversation.id, content, controller.signal)) { + for await (const event of this.source.run(conversation.id, content, controller.signal)) { if (this.disposed || this.controller !== controller) return; const previous = run.snapshot; const next = run.receive(event); - if (next !== previous) publishRun(previous, next); + if (next !== previous) applyRunTransition(previous, next); if (next.outcome) break; } @@ -247,7 +227,7 @@ export class AssistantConversationSession { const previous = run.snapshot; const next = run.interrupt(error); - if (next !== previous) publishRun(previous, next); + if (next !== previous) applyRunTransition(previous, next); } finally { if (this.controller === controller) this.controller = undefined; } @@ -264,6 +244,32 @@ export class AssistantConversationSession { await this.refreshHistory(); } + private async createConversation(controller: AbortController): Promise { + try { + const conversation = await this.source.api.createConversation({}); + if (this.disposed || this.controller !== controller) return undefined; + + // A rejected first send must reuse the conversation already created. + this.conversation = conversation; + this.phase = 'pending'; + this.entry = this.project(); + this.confirmed(this, conversation); + return conversation; + } catch (error) { + if (!this.disposed && this.controller === controller) { + this.controller = undefined; + this.phase = 'idle'; + this.failure = { + message: error instanceof Error ? error.message : 'Could not create a conversation.', + kind: 'failed', + recovery: 'send' + }; + this.publish(); + } + return undefined; + } + } + async delete(): Promise { if (this.disposed || !this.conversation || this.deleting) return { status: 'ignored' }; diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx index 0f7642d0a02..6cf6198b33e 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx @@ -1981,4 +1981,24 @@ describe('useAssistantPanel', () => { expect(panel.result.current.entry.conversation).toBeUndefined(); expect(harness.api.getMessages).toHaveBeenCalledTimes(1); }); + + it('does not submit after disposal between conversation confirmation and the send continuation', async () => { + const harness = sourceHarness(); + const run = vi.fn(async function* (): AsyncIterable { + yield { type: 'run.started' }; + yield { type: 'run.completed' }; + }); + const model = new AssistantPanelModel({ ...harness.source, run }); + model.start(); + const unsubscribe = model.subscribe(() => { + if (model.getSnapshot().entry.conversation?.id === 'created') queueMicrotask(() => model.dispose()); + }); + + await model.send('Do not submit after disposal'); + unsubscribe(); + model.dispose(); + + expect(harness.api.createConversation).toHaveBeenCalledTimes(1); + expect(run).not.toHaveBeenCalled(); + }); }); diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.spec.tsx index 633a4322d09..2f1194b7ace 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.spec.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.spec.tsx @@ -511,7 +511,7 @@ describe('AssistantConversationPanel', () => { expect(screen.getByRole('table')).toBeTruthy(); }); - it('offers the existing explicit Retry after an accepted failure without requiring a history reload', async () => { + it('retries the edited draft after an accepted failure without requiring a history reload', async () => { const source = fixture('run-failure'); const run = vi.spyOn(source, 'run'); const getMessages = vi.spyOn(source.api, 'getMessages'); @@ -525,11 +525,51 @@ describe('AssistantConversationPanel', () => { expect(run).toHaveBeenCalledTimes(1); expect(getMessages).not.toHaveBeenCalled(); + const failedAnswer = within(messages()).getByRole('article', { name: /^Assistant answer$/ }); + + fireEvent.change(input(), { target: { value: 'Read only the Europe revenue data' } }); + + expect(failedAnswer.isConnected).toBe(true); + expect(failedAnswer.textContent).toContain('stopped while reading'); + fireEvent.click(screen.getByRole('button', { name: 'Retry' })); await flush(); expect(run).toHaveBeenCalledTimes(2); - expect(run.mock.calls.map(([id]) => id)).toEqual(['c4', 'c4']); + expect(run.mock.calls.map(([id, question]) => [id, question])).toEqual([ + ['c4', 'Read the revenue data'], + ['c4', 'Read only the Europe revenue data'] + ]); + expect(within(messages()).getByText('Read only the Europe revenue data', { exact: true })).toBeTruthy(); + expect(screen.getByRole('table')).toBeTruthy(); + expect(input().value).toBe(''); + }); + + it('uses the current source for answer recovery after the panel source changes', async () => { + const first = fixture('run-failure'); + const second = fixture('run-failure'); + const firstRun = vi.spyOn(first, 'run'); + const secondRun = vi.spyOn(second, 'run'); + const view = render(); + await flush(); + await submit('Question for the first source'); + + view.rerender(); + await flush(); + await submit('Question for the second source'); + fireEvent.change(input(), { target: { value: 'Retry in the second source' } }); + + fireEvent.click(screen.getByRole('button', { name: 'Retry' })); + await flush(); + + expect(firstRun).toHaveBeenCalledTimes(1); + expect(secondRun.mock.calls.map(([, question]) => question)).toEqual([ + 'Question for the second source', + 'Retry in the second source' + ]); + expect(within(messages()).getByText('Retry in the second source', { exact: true })).toBeTruthy(); + expect(within(messages()).queryByText('Question for the first source', { exact: true })).toBeNull(); + expect(screen.getByRole('table')).toBeTruthy(); }); it('keeps a late A stream out of B and returns to A without starting a second consumer', async () => { diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.tsx index e163bbdb9f6..e59f5fa28d6 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.tsx @@ -31,7 +31,7 @@ const ignoreSearchChange = () => undefined; export const AssistantConversationPanel = ({ source, now }: AssistantConversationPanelProps) => { const panel = useAssistantPanel(source); - const { entry } = panel; + const { entry, send: sendQuestion, reload: reloadHistory } = panel; const [listOpen, setListOpen] = useState(false); const listTrigger = useRef(null); @@ -72,14 +72,14 @@ export const AssistantConversationPanel = ({ source, now }: AssistantConversatio const busy = entry.phase === 'creating' || entry.phase === 'pending' || entry.phase === 'running'; - const recoveryActions = { - send: () => { - void panel.send(); - }, - reload: () => { - void panel.reload(); - } - }; + const retrySend = useCallback(() => { + void sendQuestion(); + }, [sendQuestion]); + const retryReload = useCallback(() => { + void reloadHistory(); + }, [reloadHistory]); + + const recoveryActions = { send: retrySend, reload: retryReload }; const retry = entry.failure?.recovery ? recoveryActions[entry.failure.recovery] : undefined; const answerRetry = entry.answerRecovery ? recoveryActions[entry.answerRecovery] : undefined; diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/ConversationMessages.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/ConversationMessages.tsx index ebad2d4189d..a777901bb94 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/ConversationMessages.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/ConversationMessages.tsx @@ -39,25 +39,44 @@ export const ConversationMessages = ({ }) => { const { messages, messageKeys, localAnswerViews, answer, answerKey } = entry; - const rows = useMemo( - () => - messages.map(message => ({ - key: messageKeys.get(message.id) ?? message.id, - message - })), - [messages, messageKeys] - ); + const messageItems = useMemo(() => { + const items = messages.map(message => { + const key = messageKeys.get(message.id) ?? message.id; + return ( +
+ {message.role === 'user' ? ( + + ) : ( + + )} +
+ ); + }); + + if (answer) { + items.push( +
+ +
+ ); + } + + return items; + }, [messages, messageKeys, localAnswerViews, answer, answerKey, onRetry]); const completedMessage = answer?.status === 'done' ? answer.message : undefined; const rowSegments = useMemo(() => { const segments = new Map(); - for (const { key, message } of rows) { - if (message.role === 'assistant') segments.set(key, message.segments); + for (const message of messages) { + if (message.role === 'assistant') segments.set(messageKeys.get(message.id) ?? message.id, message.segments); } if (answerKey && completedMessage) segments.set(answerKey, completedMessage.segments); return segments; - }, [rows, answerKey, completedMessage]); + }, [messages, messageKeys, answerKey, completedMessage]); const { viewport, content, onScroll } = useConversationScroll({ scope, @@ -67,27 +86,6 @@ export const ConversationMessages = ({ loadEarlier }); - const messageItems = rows.map(({ key, message }) => ( -
- {message.role === 'user' ? ( - - ) : ( - - )} -
- )); - - if (answer) { - messageItems.push( -
- -
- ); - } - return (
Date: Sun, 25 Oct 2026 23:42:28 +0900 Subject: [PATCH 10/10] [ZEPPELIN-6748] Require initial history before sending in existing conversations --- .../assistant/ui/AssistantComposer.spec.tsx | 44 ++ .../assistant/ui/AssistantComposer.tsx | 5 +- .../model/assistantConversation.ts | 5 +- .../model/useAssistantPanel.spec.tsx | 588 ++++++++++-------- .../ui/AssistantConversationPanel.spec.tsx | 143 ++++- .../ui/AssistantConversationPanel.tsx | 1 + 6 files changed, 522 insertions(+), 264 deletions(-) diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantComposer.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantComposer.spec.tsx index 1e0bc0d865b..7c8fa0ad276 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantComposer.spec.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantComposer.spec.tsx @@ -75,6 +75,50 @@ describe('AssistantComposer', () => { expect(onSubmit).not.toHaveBeenCalled(); }); + it('keeps a blocked draft editable and submits its edited value only after submission is enabled', () => { + const onChange = vi.fn(); + const onSubmit = vi.fn(); + const { rerender } = render( + + ); + const field = input(); + + expect(field.disabled).toBe(false); + expect(field.readOnly).toBe(false); + expect(send().disabled).toBe(true); + expect(screen.queryByRole('note')).toBeNull(); + + fireEvent.change(field, { target: { value: ' Edited question ' } }); + + expect(onChange).toHaveBeenCalledExactlyOnceWith(' Edited question '); + + rerender(); + fireEvent.click(send()); + pressEnter(); + fireEvent.submit(field.form!); + + expect(onSubmit).not.toHaveBeenCalled(); + expect(input()).toBe(field); + expect(field.value).toBe(' Edited question '); + expect(field.disabled).toBe(false); + expect(field.readOnly).toBe(false); + + rerender( + + ); + + expect(input()).toBe(field); + expect(field.value).toBe(' Edited question '); + expect(send().disabled).toBe(false); + + fireEvent.click(send()); + pressEnter(); + fireEvent.submit(field.form!); + + expect(onSubmit.mock.calls).toEqual([['Edited question'], ['Edited question'], ['Edited question']]); + expect(field.value).toBe(' Edited question '); + }); + it('keeps the question visible but does not send it again while sending', () => { const { onSubmit } = renderComposer({ value: 'Explain this notebook', sending: true }); expect(input().readOnly).toBe(true); diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantComposer.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantComposer.tsx index c07d677914e..de556bd9151 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantComposer.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/AssistantComposer.tsx @@ -23,6 +23,8 @@ export interface AssistantComposerProps { /** The question cannot be sent here, for example in a conversation started by another user. */ readOnly?: boolean; readOnlyReason?: string; + /** Prevents submission while the question remains editable. */ + submitDisabled?: boolean; /** A question is on its way: the input keeps its text but cannot be changed or sent again. */ sending?: boolean; } @@ -33,12 +35,13 @@ export const AssistantComposer = ({ onSubmit, readOnly = false, readOnlyReason = "You can't send messages to this conversation.", + submitDisabled = false, sending = false }: AssistantComposerProps) => { const hintId = useId(); const question = value.trim(); // The server rejects a blank question, and it accepts one question per conversation at a time. - const canSubmit = question !== '' && !readOnly && !sending; + const canSubmit = question !== '' && !readOnly && !sending && !submitDisabled; const submit = () => { if (canSubmit) onSubmit(question); diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts index c6b5781b0fe..7c9aa3d4f68 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/assistantConversation.ts @@ -163,6 +163,7 @@ export class AssistantConversationSession { return result.status === 'committed'; } + /** Existing conversations must load their initial history page before sending. */ async send(question?: string): Promise { const submitted = question ?? this.draft; const content = submitted.trim(); @@ -172,6 +173,7 @@ export class AssistantConversationSession { !content || (this.phase !== 'idle' && this.phase !== 'unknown') || this.deleting || + (this.conversation && !this.history.snapshot.loaded) || this.conversation?.canSendMessage === false ) { return; @@ -235,8 +237,7 @@ export class AssistantConversationSession { if (this.disposed) return; // The next send must wait for this consumer to release the conversation. - this.historyRefreshPending ||= - run.snapshot.outcome === 'completed' || (run.snapshot.outcome === 'failed' && !this.history.snapshot.loaded); + this.historyRefreshPending ||= run.snapshot.outcome === 'completed'; this.phase = run.snapshot.outcome === 'unknown' ? 'unknown' : 'idle'; this.publish(); diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx index 6cf6198b33e..452691f4397 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/model/useAssistantPanel.spec.tsx @@ -914,35 +914,108 @@ describe('useAssistantPanel', () => { expect(panel.result.current.entry.loadingEarlier).toBe(false); }); - it('allows sending during initial history loading and ignores that old latest page', async () => { + it('blocks direct sends before initial history succeeds without changing the draft or invalidating the read', async () => { const harness = sourceHarness(); const history = deferred(); harness.api.getMessages.mockImplementationOnce(() => history.promise); const panel = await mountPanel(harness.source); - act(() => panel.result.current.select('a')); + act(() => panel.result.current.setDraft('Keep this draft')); + const entry = panel.result.current.entry; - expect(panel.result.current.entry.loading).toBe(true); - - act(() => panel.result.current.setDraft('Question during history loading')); - const sending = await sendQuestion(panel); + await act(async () => panel.result.current.send('Do not replace the draft')); + await act(async () => panel.result.current.send()); - expect(panel.result.current.entry).toMatchObject({ phase: 'pending', loading: false, loadingEarlier: false }); + expect(panel.result.current.entry).toBe(entry); + expect(panel.result.current.entry).toMatchObject({ + loaded: false, + loading: true, + phase: 'idle', + draft: 'Keep this draft', + messages: [] + }); + expect(harness.run).not.toHaveBeenCalled(); + expect(harness.api.createConversation).not.toHaveBeenCalled(); + expect(harness.api.getMessages.mock.calls).toEqual([['a']]); await act(async () => - harness.feeds[0].push( - { type: 'run.started' }, - { type: 'message.delta', messageId: 'current-answer', delta: 'Current answer' } - ) - ); - await act(async () => - history.resolve({ messages: [{ id: 'stale-history', role: 'user', content: 'Old history' }], cursor: null }) + history.resolve({ messages: [{ id: 'saved-question', role: 'user', content: 'Existing history' }], cursor: null }) ); + expect(panel.result.current.entry).toMatchObject({ + loaded: true, + loading: false, + phase: 'idle', + draft: 'Keep this draft', + messages: [{ id: 'saved-question', role: 'user', content: 'Existing history' }] + }); + expect(harness.api.getMessages).toHaveBeenCalledTimes(1); + }); + + it('allows sending after the latest page succeeds without loading its earlier cursor', async () => { + const harness = sourceHarness(); + const saved: AssistantMessagePage = { + messages: [ + { id: 'saved-answer', role: 'assistant', segments: [{ id: 'saved-round', content: 'Latest answer' }] } + ], + cursor: 'older-question' + }; + harness.api.getMessages.mockResolvedValueOnce(saved); + const panel = await mountPanel(harness.source); + await act(async () => panel.result.current.select('a')); + + const sending = await sendQuestion(panel, 'Next question'); + await act(async () => harness.feeds[0].push({ type: 'run.started' })); + + expect(harness.api.getMessages.mock.calls).toEqual([['a']]); + expect(harness.api.createConversation).not.toHaveBeenCalled(); + expect(harness.run).toHaveBeenCalledWith('a', 'Next question', expect.any(AbortSignal)); + expect(panel.result.current.entry).toMatchObject({ loaded: true, cursor: 'older-question', phase: 'running' }); expect(panel.result.current.entry.messages).toEqual([ - expect.objectContaining({ role: 'user', content: 'Question during history loading' }) + ...saved.messages, + expect.objectContaining({ role: 'user', content: 'Next question' }) ]); - expect(panel.result.current.entry.answer).toMatchObject({ status: 'streaming', content: 'Current answer' }); + + await act(async () => { + harness.feeds[0].push({ type: 'run.failed', message: 'Stopped' }); + await sending.completion; + }); + }); + + it('allows sending during a later latest-page reload and ignores its stale result', async () => { + const harness = sourceHarness(); + const history = deferred(); + const saved: AssistantMessagePage = { + messages: [{ id: 'saved-question', role: 'user', content: 'Existing history' }], + cursor: 'earlier' + }; + harness.api.getMessages.mockResolvedValueOnce(saved).mockImplementationOnce(() => history.promise); + const panel = await mountPanel(harness.source); + await act(async () => panel.result.current.select('a')); + let reload!: Promise; + act(() => { + reload = panel.result.current.reload(); + }); + + expect(panel.result.current.entry).toMatchObject({ loaded: true, loading: true }); + + const sending = await sendQuestion(panel, 'Question during reload'); + + expect(harness.run).toHaveBeenCalledWith('a', 'Question during reload', expect.any(AbortSignal)); + expect(panel.result.current.entry).toMatchObject({ phase: 'pending', loading: false }); + + await act(async () => { + history.resolve({ messages: [{ id: 'stale-question', role: 'user', content: 'Stale history' }], cursor: null }); + await reload; + }); + + expect(panel.result.current.entry).toMatchObject({ + loaded: true, + messages: saved.messages, + cursor: 'earlier', + draft: 'Question during reload', + phase: 'pending' + }); await act(async () => { harness.feeds[0].push({ type: 'run.failed', message: 'Stopped' }); @@ -1341,9 +1414,10 @@ describe('useAssistantPanel', () => { model.dispose(); }); - it('keeps inline answer recovery independent of a failed history reload', async () => { + it('keeps a loaded failed answer after a later reload fails and permits the next send', async () => { const harness = sourceHarness(); const panel = await mountPanel(harness.source); + await act(async () => panel.result.current.select('a')); const sending = await sendQuestion(panel, 'Accepted but failed'); await act(async () => { harness.feeds[0].push( @@ -1359,6 +1433,7 @@ describe('useAssistantPanel', () => { await act(async () => panel.result.current.reload()); expect(panel.result.current.entry).toMatchObject({ + loaded: true, phase: 'idle', failure: { message: 'History unavailable', kind: 'connection', recovery: 'reload' }, answerRecovery: 'send', @@ -1366,6 +1441,24 @@ describe('useAssistantPanel', () => { }); expect(await panel.result.current.loadEarlier()).toBe(false); + + const next = await sendQuestion(panel, 'Next question'); + + expect(harness.api.createConversation).not.toHaveBeenCalled(); + expect(harness.run.mock.calls.map(([id, content]) => [id, content])).toEqual([ + ['a', 'Accepted but failed'], + ['a', 'Next question'] + ]); + expect(panel.result.current.entry.failure).toBeUndefined(); + expect(panel.result.current.entry.messages).toMatchObject([ + { role: 'user', content: 'Accepted but failed' }, + { id: 'partial', role: 'assistant', segments: [{ id: 'partial', content: 'Retained partial' }] } + ]); + + await act(async () => { + harness.feeds[1].push({ type: 'run.failed', message: 'Next question rejected' }); + await next.completion; + }); }); it('renders a fresh initial snapshot synchronously when the source identity changes', async () => { @@ -1444,61 +1537,57 @@ describe('useAssistantPanel', () => { } ); - it('restores initial history after a rejected send without losing its error or draft', async () => { + it('keeps a failed initial read and draft unchanged by send until a successful history retry', async () => { const harness = sourceHarness(); - const initial = deferred(); - const saved: AssistantMessagePage = { - messages: [{ id: 'saved-question', role: 'user', content: 'Existing question' }], - cursor: 'earlier' - }; - harness.api.getMessages.mockImplementationOnce(() => initial.promise).mockResolvedValueOnce(saved); + const history = deferred(); + harness.api.getMessages.mockImplementationOnce(() => history.promise); const panel = await mountPanel(harness.source); act(() => panel.result.current.select('a')); - const sending = await sendQuestion(panel, 'Retry this question'); + act(() => panel.result.current.setDraft('Keep this question')); - await act(async () => { - harness.feeds[0].push({ type: 'run.failed', message: 'Message rejected' }); - await sending.completion; - initial.resolve({ messages: [], cursor: null }); - }); + await act(async () => history.reject(new Error('History unavailable'))); - expect(panel.result.current.entry).toMatchObject({ - loaded: true, - loading: false, - messages: saved.messages, - cursor: 'earlier', - draft: 'Retry this question', - failure: { message: 'Message rejected', recovery: 'send' } - }); - }); - - it('offers history recovery when restoring initial history after rejection fails', async () => { - const harness = sourceHarness(); - const initial = deferred(); - harness.api.getMessages - .mockImplementationOnce(() => initial.promise) - .mockRejectedValueOnce(new Error('History unavailable')); - const panel = await mountPanel(harness.source); - act(() => panel.result.current.select('a')); - const sending = await sendQuestion(panel, 'Keep this question'); + const entry = panel.result.current.entry; - await act(async () => { - harness.feeds[0].push({ type: 'run.failed', message: 'Message rejected' }); - await sending.completion; - initial.resolve({ messages: [], cursor: null }); - }); + await act(async () => panel.result.current.send('Do not replace the draft')); + await act(async () => panel.result.current.send()); + expect(panel.result.current.entry).toBe(entry); expect(panel.result.current.entry).toMatchObject({ + loaded: false, + loading: false, + phase: 'idle', draft: 'Keep this question', - failure: { message: 'History unavailable', recovery: 'reload' } + failure: { message: 'History unavailable', kind: 'connection', recovery: 'reload' } }); + expect(harness.api.getMessages.mock.calls).toEqual([['a']]); + expect(harness.api.createConversation).not.toHaveBeenCalled(); + expect(harness.run).not.toHaveBeenCalled(); + harness.api.getMessages.mockResolvedValueOnce({ messages: [{ id: 'saved', role: 'user', content: 'Recovered history' }], cursor: null }); await act(async () => panel.result.current.reload()); - expect(panel.result.current.entry.messages).toEqual([{ id: 'saved', role: 'user', content: 'Recovered history' }]); - expect(panel.result.current.entry.draft).toBe('Keep this question'); + + expect(panel.result.current.entry).toMatchObject({ + loaded: true, + draft: 'Keep this question', + messages: [{ id: 'saved', role: 'user', content: 'Recovered history' }] + }); + expect(panel.result.current.entry.failure).toBeUndefined(); + expect(harness.api.getMessages.mock.calls).toEqual([['a'], ['a']]); + expect(harness.run).not.toHaveBeenCalled(); + + const sending = await sendQuestion(panel); + + expect(harness.run).toHaveBeenCalledWith('a', 'Keep this question', expect.any(AbortSignal)); + expect(harness.api.createConversation).not.toHaveBeenCalled(); + + await act(async () => { + harness.feeds[0].push({ type: 'run.failed', message: 'Stopped' }); + await sending.completion; + }); }); it('refreshes activity metadata and ordering after a send in an existing conversation', async () => { @@ -1709,228 +1798,229 @@ describe('useAssistantPanel', () => { expect(panel.result.current.entry.draft).toBe('Next A question'); }); - it.each([false, true])( - 'recovers interrupted initial history without losing a failed answer, persisted partial=%s', - async persisted => { - const harness = sourceHarness(); - const initial = Promise.withResolvers(); - const content = 'Repeated question'; - const questions: AssistantMessagePage['messages'] = [ - { id: 'older-question', role: 'user', content }, - { id: 'older-answer', role: 'assistant', segments: [{ id: 'older-round', content: 'Older answer' }] }, - { id: 'saved-question', role: 'user', content } - ]; - harness.api.getMessages - .mockImplementationOnce(() => initial.promise) - .mockResolvedValueOnce({ - messages: persisted - ? [...questions, { id: 'saved-answer', role: 'assistant', segments: [{ id: 'round', content: 'Partial' }] }] - : questions, - cursor: 'earlier' - }); - const panel = await mountPanel(harness.source); - act(() => panel.result.current.select('a')); - const sending = await sendQuestion(panel, content); - await act(async () => - harness.feeds[0].push( - { type: 'run.started' }, - { type: 'message.done', messageId: 'round', content: 'Partial' }, - { type: 'tool_call.started', toolCallId: 'tool', name: 'read_note' }, - { type: 'tool_call.done', toolCallId: 'tool' }, - { type: 'message.delta', messageId: 'continuation', delta: 'Unsaved continuation' } - ) - ); - const answerKey = panel.result.current.entry.answerKey; - const questionKey = panel.result.current.entry.messages.at(-1)!.id; - - await act(async () => { - harness.feeds[0].push({ type: 'run.failed', message: 'Execution failed' }); - await sending.completion; - initial.resolve({ messages: [], cursor: 'obsolete' }); + it.each([false, true])('reloads history without losing a failed answer, persisted partial=%s', async persisted => { + const harness = sourceHarness(); + const content = 'Repeated question'; + const questions: AssistantMessagePage['messages'] = [ + { id: 'older-question', role: 'user', content }, + { id: 'older-answer', role: 'assistant', segments: [{ id: 'older-round', content: 'Older answer' }] }, + { id: 'saved-question', role: 'user', content } + ]; + harness.api.getMessages + .mockResolvedValueOnce({ messages: questions.slice(0, -1), cursor: 'earlier' }) + .mockResolvedValueOnce({ + messages: persisted + ? [...questions, { id: 'saved-answer', role: 'assistant', segments: [{ id: 'round', content: 'Partial' }] }] + : questions, + cursor: 'earlier' }); + const panel = await mountPanel(harness.source); + await act(async () => panel.result.current.select('a')); + const sending = await sendQuestion(panel, content); + await act(async () => + harness.feeds[0].push( + { type: 'run.started' }, + { type: 'message.done', messageId: 'round', content: 'Partial' }, + { type: 'tool_call.started', toolCallId: 'tool', name: 'read_note' }, + { type: 'tool_call.done', toolCallId: 'tool' }, + { type: 'message.delta', messageId: 'continuation', delta: 'Unsaved continuation' } + ) + ); + const answerKey = panel.result.current.entry.answerKey; + const questionKey = panel.result.current.entry.messages.at(-1)!.id; - expect(panel.result.current.entry).toMatchObject({ - loaded: true, - loading: false, - cursor: 'earlier', - draft: content, - answerKey, - answerRecovery: 'send', - answer: { - status: 'error', - message: 'Execution failed', - content: 'Partial\n\nUnsaved continuation', - tools: [{ id: 'tool', name: 'read_note', status: 'done' }] - } - }); - expect(panel.result.current.entry.messages).toEqual(questions); - expect(panel.result.current.entry.messageKeys.get('saved-question')).toBe(questionKey); - expect(harness.run).toHaveBeenCalledTimes(1); - } - ); + await act(async () => { + harness.feeds[0].push({ type: 'run.failed', message: 'Execution failed' }); + await sending.completion; + }); - it.each([false, true])( - 'resumes deferred completion history after deletion fails, initial history loaded=%s', - async loaded => { - const harness = sourceHarness(); - const initial = Promise.withResolvers(); - const canonical = Promise.withResolvers(); - const deletion = Promise.withResolvers(); - const saved: AssistantMessagePage = { - messages: [{ id: 'earlier-question', role: 'user', content: 'Earlier question' }], - cursor: 'earlier' - }; - harness.api.getMessages - .mockImplementationOnce(() => (loaded ? Promise.resolve(saved) : initial.promise)) - .mockImplementationOnce(() => canonical.promise); - harness.api.deleteConversation.mockImplementationOnce(() => deletion.promise); - const panel = await mountPanel(harness.source); - await act(async () => panel.result.current.select('a')); - const sending = await sendQuestion(panel, 'Complete this question'); - await act(async () => - harness.feeds[0].push( - { type: 'run.started' }, - { type: 'message.done', messageId: 'round', content: 'Completed answer' } - ) - ); - let removing!: Promise; - act(() => { - removing = panel.result.current.deleteConversation('a'); - }); + await act(async () => panel.result.current.reload()); - await act(async () => { - harness.feeds[0].push({ type: 'run.completed' }); - await sending.completion; - }); + expect(panel.result.current.entry).toMatchObject({ + loaded: true, + loading: false, + cursor: 'earlier', + draft: content, + answerKey, + answerRecovery: 'send', + answer: { + status: 'error', + message: 'Execution failed', + content: 'Partial\n\nUnsaved continuation', + tools: [{ id: 'tool', name: 'read_note', status: 'done' }] + } + }); + expect(panel.result.current.entry.messages).toEqual(questions); + expect(panel.result.current.entry.messageKeys.get('saved-question')).toBe(questionKey); + expect(harness.run).toHaveBeenCalledTimes(1); + }); + + it('resumes deferred completion history after deletion fails', async () => { + const harness = sourceHarness(); + const canonical = Promise.withResolvers(); + const deletion = Promise.withResolvers(); + const saved: AssistantMessagePage = { + messages: [{ id: 'earlier-question', role: 'user', content: 'Earlier question' }], + cursor: 'earlier' + }; + harness.api.getMessages.mockResolvedValueOnce(saved).mockImplementationOnce(() => canonical.promise); + harness.api.deleteConversation.mockImplementationOnce(() => deletion.promise); + const panel = await mountPanel(harness.source); + await act(async () => panel.result.current.select('a')); + const sending = await sendQuestion(panel, 'Complete this question'); + await act(async () => + harness.feeds[0].push( + { type: 'run.started' }, + { type: 'message.done', messageId: 'round', content: 'Completed answer' } + ) + ); + let removing!: Promise; + act(() => { + removing = panel.result.current.deleteConversation('a'); + }); - expect(harness.api.getMessages).toHaveBeenCalledTimes(1); - expect(panel.result.current.entry).toMatchObject({ deleting: true, answer: { status: 'done' } }); + await act(async () => { + harness.feeds[0].push({ type: 'run.completed' }); + await sending.completion; + }); - let result!: AssistantDeletionResult; - await act(async () => { - deletion.reject(new Error('Deletion denied')); - result = await removing; - }); + expect(harness.api.getMessages).toHaveBeenCalledTimes(1); + expect(panel.result.current.entry).toMatchObject({ deleting: true, answer: { status: 'done' } }); - expect(result).toEqual({ status: 'failed', message: 'Deletion denied' }); - expect(panel.result.current.entry).toMatchObject({ - deleting: false, - loading: true, - failure: { message: 'Deletion denied', kind: 'failed' } - }); - const page: AssistantMessagePage = { + let result!: AssistantDeletionResult; + await act(async () => { + deletion.reject(new Error('Deletion denied')); + result = await removing; + }); + + expect(result).toEqual({ status: 'failed', message: 'Deletion denied' }); + expect(panel.result.current.entry).toMatchObject({ + deleting: false, + loading: true, + failure: { message: 'Deletion denied', kind: 'failed' } + }); + const page: AssistantMessagePage = { + messages: [ + ...saved.messages, + { id: 'saved-question', role: 'user', content: 'Complete this question' }, + { id: 'saved-answer', role: 'assistant', segments: [{ id: 'round', content: 'Completed answer' }] } + ], + cursor: 'earlier' + }; + + await act(async () => canonical.resolve(page)); + + expect(panel.result.current.entry).toMatchObject({ + loaded: true, + loading: false, + messages: page.messages, + cursor: 'earlier', + failure: { message: 'Deletion denied', kind: 'failed' } + }); + expect(panel.result.current.entry.answer).toBeUndefined(); + expect(harness.api.getMessages).toHaveBeenCalledTimes(2); + expect(harness.run).toHaveBeenCalledTimes(1); + }); + + it('preserves a recovered failed turn while loading distinct older history and promotes its partial once', async () => { + const harness = sourceHarness(); + harness.api.getMessages + .mockResolvedValueOnce({ + messages: [{ id: 'loaded-question', role: 'user', content: 'Previously loaded question' }], + cursor: 'before-loaded-question' + }) + .mockResolvedValueOnce({ messages: [ - ...saved.messages, - { id: 'saved-question', role: 'user', content: 'Complete this question' }, - { id: 'saved-answer', role: 'assistant', segments: [{ id: 'round', content: 'Completed answer' }] } + { id: 'loaded-question', role: 'user', content: 'Previously loaded question' }, + { id: 'saved-question', role: 'user', content: 'Failed question' }, + { id: 'saved-answer', role: 'assistant', segments: [{ id: 'round', content: 'Partial' }] } ], - cursor: 'earlier' - }; - - await act(async () => { - canonical.resolve(page); - initial.resolve({ messages: [], cursor: 'obsolete' }); + cursor: 'before-loaded-question' + }) + .mockResolvedValueOnce({ + messages: [ + { id: 'oldest-question', role: 'user', content: 'Oldest question' }, + { id: 'oldest-answer', role: 'assistant', segments: [{ id: 'oldest-round', content: 'Oldest answer' }] } + ], + cursor: null }); + const panel = await mountPanel(harness.source); + await act(async () => panel.result.current.select('a')); + const sending = await sendQuestion(panel, 'Failed question'); + await act(async () => { + harness.feeds[0].push( + { type: 'run.started' }, + { type: 'message.delta', messageId: 'round', delta: 'Partial' }, + { type: 'run.failed', message: 'Execution failed' } + ); + await sending.completion; + }); + const answerKey = panel.result.current.entry.answerKey; + const questionKey = panel.result.current.entry.messages.at(-1)!.id; - expect(panel.result.current.entry).toMatchObject({ - loaded: true, - loading: false, - messages: page.messages, - cursor: 'earlier', - failure: { message: 'Deletion denied', kind: 'failed' } - }); - expect(panel.result.current.entry.answer).toBeUndefined(); - expect(harness.api.getMessages).toHaveBeenCalledTimes(2); - expect(harness.run).toHaveBeenCalledTimes(1); - } - ); + await act(async () => panel.result.current.reload()); - it.each([false, true])( - 'joins the retained failed turn when the earlier page includes a partial=%s', - async includePartial => { - const harness = sourceHarness(); - const initial = Promise.withResolvers(); - harness.api.getMessages - .mockImplementationOnce(() => initial.promise) - .mockResolvedValueOnce({ - messages: [{ id: 'saved-answer', role: 'assistant', segments: [{ id: 'round', content: 'Partial' }] }], - cursor: 'earlier' - }) - .mockResolvedValueOnce({ - messages: [ - { id: 'older-question', role: 'user', content: 'Earlier question' }, - { id: 'saved-question', role: 'user', content: 'Failed question' }, - ...(includePartial - ? [{ id: 'saved-prefix', role: 'assistant' as const, segments: [{ id: 'round', content: 'Partial' }] }] - : []) - ], - cursor: null - }); - const panel = await mountPanel(harness.source); - act(() => panel.result.current.select('a')); - const sending = await sendQuestion(panel, 'Failed question'); - await act(async () => { - harness.feeds[0].push( - { type: 'run.started' }, - { type: 'message.delta', messageId: 'round', delta: 'Partial' }, - { type: 'run.failed', message: 'Execution failed' } - ); - await sending.completion; - }); - const answerKey = panel.result.current.entry.answerKey; - const questionKey = panel.result.current.entry.messages.at(-1)!.id; + expect(panel.result.current.entry).toMatchObject({ + cursor: 'before-loaded-question', + answerKey, + answerRecovery: 'send', + answer: { status: 'error', message: 'Execution failed', content: 'Partial' } + }); + expect(panel.result.current.entry.messages).toEqual([ + { id: 'loaded-question', role: 'user', content: 'Previously loaded question' }, + { id: 'saved-question', role: 'user', content: 'Failed question' } + ]); + expect(panel.result.current.entry.messageKeys.get('saved-question')).toBe(questionKey); - await act(async () => { - await panel.result.current.loadEarlier(); - initial.resolve({ messages: [], cursor: 'obsolete' }); - }); + await act(async () => panel.result.current.loadEarlier()); - expect(panel.result.current.entry.messages).toEqual([ - { id: 'older-question', role: 'user', content: 'Earlier question' }, - { id: 'saved-question', role: 'user', content: 'Failed question' } - ]); - expect(panel.result.current.entry.messageKeys.get('saved-question')).toBe(questionKey); - expect(panel.result.current.entry).toMatchObject({ - cursor: null, - answerKey, - answerRecovery: 'send', - answer: { status: 'error', message: 'Execution failed', content: 'Partial' } - }); + expect(harness.api.getMessages.mock.calls).toEqual([['a'], ['a'], ['a', 'before-loaded-question']]); + expect(panel.result.current.entry.messages).toEqual([ + { id: 'oldest-question', role: 'user', content: 'Oldest question' }, + { id: 'oldest-answer', role: 'assistant', segments: [{ id: 'oldest-round', content: 'Oldest answer' }] }, + { id: 'loaded-question', role: 'user', content: 'Previously loaded question' }, + { id: 'saved-question', role: 'user', content: 'Failed question' } + ]); + expect(panel.result.current.entry).toMatchObject({ + cursor: null, + answerKey, + answerRecovery: 'send', + answer: { status: 'error', message: 'Execution failed', content: 'Partial' } + }); - const retry = await sendQuestion(panel, 'Another question'); - await act(async () => { - harness.feeds[1].push({ type: 'run.failed', message: 'Not accepted' }); - await retry.completion; - }); + const retry = await sendQuestion(panel, 'Another question'); + await act(async () => { + harness.feeds[1].push({ type: 'run.failed', message: 'Not accepted' }); + await retry.completion; + }); - expect(panel.result.current.entry.messages.filter(message => message.role === 'assistant')).toMatchObject([ - { id: 'round', role: 'assistant', segments: [{ id: 'round', content: 'Partial' }] } - ]); - expect(panel.result.current.entry.localAnswerViews.get('round')).toMatchObject({ - status: 'error', - message: 'Execution failed' - }); - } - ); + expect(panel.result.current.entry.messages.filter(message => message.role === 'assistant')).toMatchObject([ + { id: 'oldest-answer', role: 'assistant', segments: [{ id: 'oldest-round', content: 'Oldest answer' }] }, + { id: 'round', role: 'assistant', segments: [{ id: 'round', content: 'Partial' }] } + ]); + expect(panel.result.current.entry.localAnswerViews.get('round')).toMatchObject({ + status: 'error', + message: 'Execution failed' + }); + }); it('keeps failed recovery available for explicit reload without retrying automatically', async () => { const harness = sourceHarness(); - const initial = Promise.withResolvers(); harness.api.getMessages - .mockImplementationOnce(() => initial.promise) + .mockResolvedValueOnce({ messages: [], cursor: null }) .mockRejectedValueOnce(new Error('History unavailable')); const panel = await mountPanel(harness.source); - act(() => panel.result.current.select('a')); + await act(async () => panel.result.current.select('a')); const sending = await sendQuestion(panel, 'Failed question'); await act(async () => { harness.feeds[0].push({ type: 'run.started' }, { type: 'run.failed', message: 'Execution failed' }); await sending.completion; - initial.resolve({ messages: [], cursor: 'obsolete' }); + await panel.result.current.reload(); }); expect(panel.result.current.entry).toMatchObject({ - loaded: false, + loaded: true, failure: { message: 'History unavailable', recovery: 'reload' }, answer: { status: 'error', message: 'Execution failed' } }); diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.spec.tsx index 2f1194b7ace..31b83daf965 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.spec.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.spec.tsx @@ -144,6 +144,7 @@ describe('AssistantConversationPanel', () => { try { const source = fixture('send-rejection'); + const run = vi.spyOn(source, 'run'); const getMessages = source.api.getMessages; let pending = true; vi.spyOn(source.api, 'getMessages').mockImplementation(async (id, before) => { @@ -158,8 +159,23 @@ describe('AssistantConversationPanel', () => { await select('Revenue analysis'); expect(source.api.getMessages).toHaveBeenCalledWith('c1', 'c1:earlier'); + expect(screen.getByText('Loading earlier messages…')).toBeTruthy(); + expect(input().disabled).toBe(false); + expect(input().readOnly).toBe(false); + + fireEvent.change(input(), { target: { value: 'A question rejected before acceptance' } }); + + expect(send().matches(':disabled')).toBe(false); + + fireEvent.click(send()); + await flush(); + + expect(run).toHaveBeenCalledExactlyOnceWith( + 'c1', + 'A question rejected before acceptance', + expect.any(AbortSignal) + ); - await submit('A question rejected before acceptance'); await act(async () => earlier.resolve()); expect(await within(messages()).findByText('I checked the regional revenue paragraph.')).toBeTruthy(); @@ -621,25 +637,70 @@ describe('AssistantConversationPanel', () => { expect(run).not.toHaveBeenCalled(); }); - it('shows a history failure and reloads explicitly without sending a question', async () => { + it('keeps a draft blocked after an initial history failure until explicit reload succeeds without resending', async () => { const source = fixture(); + const initial = deferred(); + const retry = deferred(); + const originalGetMessages = source.api.getMessages; const getMessages = vi .spyOn(source.api, 'getMessages') - .mockRejectedValueOnce(new Error('Saved history is unavailable.')); + .mockImplementationOnce(async () => { + await initial.promise; + throw new Error('Saved history is unavailable.'); + }) + .mockImplementationOnce(async id => { + await retry.promise; + return originalGetMessages(id); + }); const run = vi.spyOn(source, 'run'); await mountPanel(source); - await select('Revenue analysis'); + fireEvent.change(input(), { target: { value: 'Keep this draft after the failed lookup' } }); + await act(async () => initial.resolve()); + expect(screen.getByRole('alert').textContent).toContain('Saved history is unavailable.'); + expect(input().value).toBe('Keep this draft after the failed lookup'); + expect(input().disabled).toBe(false); + expect(input().readOnly).toBe(false); + expect(send().matches(':disabled')).toBe(true); + + fireEvent.change(input(), { target: { value: 'Edited question after lookup failure' } }); + fireEvent.click(send()); + fireEvent.keyDown(input(), { key: 'Enter' }); + fireEvent.submit(input().form!); + await flush(); + + expect(input().value).toBe('Edited question after lookup failure'); + expect(getMessages.mock.calls).toEqual([['c1']]); + expect(run).not.toHaveBeenCalled(); fireEvent.click(screen.getByRole('button', { name: 'Reload conversation' })); await flush(); expect(getMessages.mock.calls).toEqual([['c1'], ['c1']]); + expect(input().value).toBe('Edited question after lookup failure'); + expect(send().matches(':disabled')).toBe(true); + expect(run).not.toHaveBeenCalled(); + + fireEvent.keyDown(input(), { key: 'Enter' }); + fireEvent.submit(input().form!); + await flush(); + + expect(run).not.toHaveBeenCalled(); + + await act(async () => retry.resolve()); + expect(screen.getByText('Revenue increased in all three regions, led by Europe.')).toBeTruthy(); expect(screen.queryByRole('alert')).toBeNull(); + expect(input().value).toBe('Edited question after lookup failure'); + expect(send().matches(':disabled')).toBe(false); expect(run).not.toHaveBeenCalled(); + + fireEvent.click(send()); + await flush(); + + expect(run).toHaveBeenCalledExactlyOnceWith('c1', 'Edited question after lookup failure', expect.any(AbortSignal)); }); it('confirms deletion of an active conversation and lets the API decide the result', async () => { @@ -736,7 +797,7 @@ describe('AssistantConversationPanel', () => { expect(screen.getByText('Revenue increased in all three regions, led by Europe.')).toBeTruthy(); }); - it('keeps the draft editable and normal Send available while history is loading', async () => { + it('keeps the initial history draft editable but blocks every submission until the read succeeds', async () => { const source = fixture(); const history = deferred(); const originalGetMessages = source.api.getMessages; @@ -746,24 +807,46 @@ describe('AssistantConversationPanel', () => { }); const run = vi.spyOn(source, 'run'); await mountPanel(source); - await select('Revenue analysis'); + const field = input(); expect(screen.getByText('Loading conversation…')).toBeTruthy(); - expect(input().disabled).toBe(false); + expect(field.disabled).toBe(false); + expect(field.readOnly).toBe(false); + expect(screen.queryByText('Sending question…')).toBeNull(); + expect(screen.queryByText('Thinking…')).toBeNull(); - fireEvent.change(input(), { target: { value: 'Question during history lookup' } }); + fireEvent.change(field, { target: { value: 'Question during history lookup' } }); - expect(input().value).toBe('Question during history lookup'); - expect(send().matches(':disabled')).toBe(false); - expect(run).not.toHaveBeenCalled(); + expect(field.value).toBe('Question during history lookup'); + expect(send().matches(':disabled')).toBe(true); fireEvent.click(send()); + fireEvent.keyDown(field, { key: 'Enter' }); + fireEvent.submit(field.form!); await flush(); - expect(run).toHaveBeenCalledExactlyOnceWith('c1', 'Question during history lookup', expect.any(AbortSignal)); + expect(run).not.toHaveBeenCalled(); + expect(field.value).toBe('Question during history lookup'); + expect(screen.getByText('Loading conversation…')).toBeTruthy(); + fireEvent.change(field, { target: { value: 'Edited question before history arrives' } }); await act(async () => history.resolve()); + + expect(input()).toBe(field); + expect(field.value).toBe('Edited question before history arrives'); + expect(screen.getByText('Revenue increased in all three regions, led by Europe.')).toBeTruthy(); + expect(send().matches(':disabled')).toBe(false); + expect(run).not.toHaveBeenCalled(); + + fireEvent.keyDown(field, { key: 'Enter' }); + await flush(); + + expect(run).toHaveBeenCalledExactlyOnceWith( + 'c1', + 'Edited question before history arrives', + expect.any(AbortSignal) + ); }); it.each(['Cancel', 'Escape'])( @@ -898,4 +981,40 @@ describe('AssistantConversationPanel', () => { expect(within(dialog).getByRole('alert').textContent).toContain('B deletion denied'); expect(within(dialog).queryByText('A is still running')).toBeNull(); }); + + it('keeps initial-history readiness and drafts separate when switching conversations', async () => { + const source = fixture(); + const history = deferred(); + const getMessages = source.api.getMessages; + vi.spyOn(source.api, 'getMessages').mockImplementationOnce(async id => { + await history.promise; + return getMessages(id); + }); + const run = vi.spyOn(source, 'run'); + await mountPanel(source); + await select('Revenue analysis'); + fireEvent.change(input(), { target: { value: 'Keep the pending A draft' } }); + + expect(send().matches(':disabled')).toBe(true); + + await select('Notebook summary'); + fireEvent.change(input(), { target: { value: 'Question for loaded B' } }); + + expect(send().matches(':disabled')).toBe(false); + + fireEvent.click(send()); + await flush(); + await act(async () => history.resolve()); + + expect(run).toHaveBeenCalledExactlyOnceWith('c2', 'Question for loaded B', expect.any(AbortSignal)); + expect(within(messages()).getByText('Question for loaded B', { exact: true })).toBeTruthy(); + expect(within(messages()).queryByText('Keep the pending A draft', { exact: true })).toBeNull(); + expect(input().value).toBe(''); + + await select('Revenue analysis'); + + expect(input().value).toBe('Keep the pending A draft'); + expect(send().matches(':disabled')).toBe(false); + expect(run).toHaveBeenCalledTimes(1); + }); }); diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.tsx index e59f5fa28d6..a0b68e71613 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/pages/assistant-workspace/ui/AssistantConversationPanel.tsx @@ -144,6 +144,7 @@ export const AssistantConversationPanel = ({ source, now }: AssistantConversatio void panel.send(question); }} readOnly={entry.conversation?.canSendMessage === false} + submitDisabled={!!entry.conversation && !entry.loaded} sending={busy || entry.deleting} />