mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-12 18:15:33 +00:00
fix: better message input arming
This commit is contained in:
1 parent
053f6d81b9
commit
8fdbc82199
11 files changed
+911
-72
No files matched your search
@@ -10,6 +10,7 @@ import SharedAgentGate from './agents/SharedAgentGate';
|
||||
import DocsGPT3 from './assets/cute_docsgpt3.svg';
|
||||
import ActionButtons from './components/ActionButtons';
|
||||
import AdminRoute from './components/AdminRoute';
|
||||
import ErrorBoundary from './components/ErrorBoundary';
|
||||
import Spinner from './components/Spinner';
|
||||
import { Button } from './components/ui/button';
|
||||
import UploadToast from './components/UploadToast';
|
||||
@@ -75,6 +76,7 @@ function AuthWrapper({ children }: { children: React.ReactNode }) {
|
||||
function MainLayout() {
|
||||
const { isMobile, isTablet } = useMediaQuery();
|
||||
const [navOpen, setNavOpen] = useState(!(isMobile || isTablet));
|
||||
const location = useLocation();
|
||||
|
||||
return (
|
||||
<div className="bg-background relative h-screen overflow-hidden">
|
||||
@@ -87,7 +89,11 @@ function MainLayout() {
|
||||
: 'ml-0 lg:ml-16'
|
||||
}`}
|
||||
>
|
||||
<Outlet />
|
||||
{/* Contain route render crashes so navigation stays usable;
|
||||
keyed by path so the boundary resets when the user leaves. */}
|
||||
<ErrorBoundary key={location.pathname}>
|
||||
<Outlet />
|
||||
</ErrorBoundary>
|
||||
</div>
|
||||
<UploadToast />
|
||||
<ToolApprovalToast />
|
||||
|
||||
@@ -0,0 +1,102 @@
|
||||
import { act } from 'react';
|
||||
import { createRoot, type Root } from 'react-dom/client';
|
||||
|
||||
vi.mock('react-i18next', () => ({
|
||||
useTranslation: () => ({ t: (key: string) => key }),
|
||||
}));
|
||||
|
||||
import ErrorBoundary from './ErrorBoundary';
|
||||
|
||||
Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true });
|
||||
|
||||
function Bomb({ armed }: { armed: boolean }) {
|
||||
if (armed) throw new Error('render exploded');
|
||||
return <div data-testid="child">safe child</div>;
|
||||
}
|
||||
|
||||
describe('ErrorBoundary', () => {
|
||||
let container: HTMLDivElement;
|
||||
let root: Root;
|
||||
let consoleError: ReturnType<typeof vi.spyOn>;
|
||||
|
||||
beforeEach(() => {
|
||||
container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
root = createRoot(container);
|
||||
// React re-logs caught render errors; keep test output clean.
|
||||
consoleError = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await act(async () => root.unmount());
|
||||
container.remove();
|
||||
consoleError.mockRestore();
|
||||
});
|
||||
|
||||
it('renders children when nothing throws', async () => {
|
||||
await act(async () => {
|
||||
root.render(
|
||||
<ErrorBoundary>
|
||||
<Bomb armed={false} />
|
||||
</ErrorBoundary>,
|
||||
);
|
||||
});
|
||||
expect(container.querySelector('[data-testid="child"]')).not.toBeNull();
|
||||
});
|
||||
|
||||
it('contains a throwing child and shows the fallback', async () => {
|
||||
await act(async () => {
|
||||
root.render(
|
||||
<ErrorBoundary>
|
||||
<Bomb armed={true} />
|
||||
</ErrorBoundary>,
|
||||
);
|
||||
});
|
||||
expect(container.querySelector('[data-testid="child"]')).toBeNull();
|
||||
expect(container.querySelector('[role="alert"]')).not.toBeNull();
|
||||
expect(consoleError).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('recovers via the retry button once the cause is gone', async () => {
|
||||
await act(async () => {
|
||||
root.render(
|
||||
<ErrorBoundary>
|
||||
<Bomb armed={true} />
|
||||
</ErrorBoundary>,
|
||||
);
|
||||
});
|
||||
// The cause disappears (e.g. new data), but the boundary still holds.
|
||||
await act(async () => {
|
||||
root.render(
|
||||
<ErrorBoundary>
|
||||
<Bomb armed={false} />
|
||||
</ErrorBoundary>,
|
||||
);
|
||||
});
|
||||
expect(container.querySelector('[role="alert"]')).not.toBeNull();
|
||||
|
||||
const button = container.querySelector('button');
|
||||
expect(button).not.toBeNull();
|
||||
await act(async () => {
|
||||
button!.dispatchEvent(new MouseEvent('click', { bubbles: true }));
|
||||
});
|
||||
expect(container.querySelector('[data-testid="child"]')).not.toBeNull();
|
||||
});
|
||||
|
||||
it('uses a custom fallback render prop when provided', async () => {
|
||||
await act(async () => {
|
||||
root.render(
|
||||
<ErrorBoundary
|
||||
fallback={(retry) => (
|
||||
<button data-testid="custom" onClick={retry}>
|
||||
custom fallback
|
||||
</button>
|
||||
)}
|
||||
>
|
||||
<Bomb armed={true} />
|
||||
</ErrorBoundary>,
|
||||
);
|
||||
});
|
||||
expect(container.querySelector('[data-testid="custom"]')).not.toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,57 @@
|
||||
import { Component, type ErrorInfo, type ReactNode } from 'react';
|
||||
import { useTranslation } from 'react-i18next';
|
||||
|
||||
import { Button } from './ui/button';
|
||||
|
||||
type ErrorBoundaryProps = {
|
||||
children: ReactNode;
|
||||
// Render-prop fallback; receives a retry callback that re-attempts
|
||||
// rendering the children.
|
||||
fallback?: (retry: () => void) => ReactNode;
|
||||
};
|
||||
|
||||
type ErrorBoundaryState = {
|
||||
hasError: boolean;
|
||||
};
|
||||
|
||||
function DefaultFallback({ onRetry }: { onRetry: () => void }) {
|
||||
const { t } = useTranslation();
|
||||
return (
|
||||
<div
|
||||
role="alert"
|
||||
className="text-muted-foreground flex flex-col items-center gap-3 p-6 text-sm"
|
||||
>
|
||||
<p>{t('errorBoundary.message')}</p>
|
||||
<Button type="button" variant="outline" onClick={onRetry}>
|
||||
{t('errorBoundary.tryAgain')}
|
||||
</Button>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
export default class ErrorBoundary extends Component<
|
||||
ErrorBoundaryProps,
|
||||
ErrorBoundaryState
|
||||
> {
|
||||
state: ErrorBoundaryState = { hasError: false };
|
||||
|
||||
static getDerivedStateFromError(): ErrorBoundaryState {
|
||||
return { hasError: true };
|
||||
}
|
||||
|
||||
componentDidCatch(error: Error, info: ErrorInfo) {
|
||||
console.error('ErrorBoundary caught a render error:', error, info);
|
||||
}
|
||||
|
||||
retry = () => {
|
||||
this.setState({ hasError: false });
|
||||
};
|
||||
|
||||
render() {
|
||||
if (this.state.hasError) {
|
||||
if (this.props.fallback) return this.props.fallback(this.retry);
|
||||
return <DefaultFallback onRetry={this.retry} />;
|
||||
}
|
||||
return this.props.children;
|
||||
}
|
||||
}
|
||||
@@ -44,6 +44,7 @@ import {
|
||||
SourcesTrigger,
|
||||
ToolsTrigger,
|
||||
} from './message-input';
|
||||
import { useArmedSend } from './message-input/armedSend';
|
||||
import { handleAbort } from '../conversation/conversationSlice';
|
||||
import {
|
||||
AUDIO_FILE_ACCEPT_ATTR,
|
||||
@@ -1483,34 +1484,52 @@ export default function MessageInput({
|
||||
};
|
||||
|
||||
// When ``allowSendWithoutText`` is set, an attachment-only submit is
|
||||
// permitted as long as at least one attachment has finished processing.
|
||||
const hasCompletedAttachment = attachments.some(
|
||||
// permitted as long as at least one attachment exists; a still-pending
|
||||
// one arms the send instead of submitting (see handleSubmit).
|
||||
const completedAttachmentCount = attachments.filter(
|
||||
(attachment) => attachment.status === 'completed',
|
||||
);
|
||||
).length;
|
||||
const hasSubmittableContent =
|
||||
Boolean(value.trim()) || (allowSendWithoutText && hasCompletedAttachment);
|
||||
Boolean(value.trim()) || (allowSendWithoutText && attachments.length > 0);
|
||||
const canSubmit =
|
||||
hasSubmittableContent &&
|
||||
!loading &&
|
||||
recordingState !== 'recording' &&
|
||||
recordingState !== 'transcribing';
|
||||
|
||||
const handleSubmit = () => {
|
||||
if (canSubmit) {
|
||||
onSubmit(value);
|
||||
setValue('');
|
||||
if (isTouch) {
|
||||
inputRef.current?.blur();
|
||||
} else if (autoFocus) {
|
||||
setTimeout(() => {
|
||||
if (isMountedRef.current) {
|
||||
inputRef.current?.focus();
|
||||
}
|
||||
}, 0);
|
||||
}
|
||||
const submitNow = () => {
|
||||
onSubmit(value);
|
||||
setValue('');
|
||||
if (isTouch) {
|
||||
inputRef.current?.blur();
|
||||
} else if (autoFocus) {
|
||||
setTimeout(() => {
|
||||
if (isMountedRef.current) {
|
||||
inputRef.current?.focus();
|
||||
}
|
||||
}, 0);
|
||||
}
|
||||
};
|
||||
|
||||
const {
|
||||
armed: sendArmed,
|
||||
readiness: sendReadiness,
|
||||
arm: armSend,
|
||||
cancel: cancelArmedSend,
|
||||
} = useArmedSend({ attachments, onFlush: submitNow });
|
||||
|
||||
const handleSubmit = () => {
|
||||
if (!canSubmit) return;
|
||||
// Attachments still uploading/parsing (or failed) must never be
|
||||
// silently dropped from the payload: hold the send in the composer
|
||||
// until every attachment resolves, then flush automatically.
|
||||
if (sendReadiness.state !== 'ready') {
|
||||
armSend();
|
||||
return;
|
||||
}
|
||||
submitNow();
|
||||
};
|
||||
|
||||
const handleCancel = () => {
|
||||
handleAbort();
|
||||
};
|
||||
@@ -1571,6 +1590,42 @@ export default function MessageInput({
|
||||
onDropOn={handleDropOn}
|
||||
/>
|
||||
|
||||
{sendArmed && sendReadiness.state === 'waiting' && (
|
||||
<div
|
||||
className="text-muted-foreground flex items-center gap-2 px-2 pb-1 text-xs sm:px-3"
|
||||
role="status"
|
||||
>
|
||||
<span>
|
||||
{t('conversation.attachments.waitingToSend', {
|
||||
count: sendReadiness.pendingCount,
|
||||
})}
|
||||
</span>
|
||||
<button
|
||||
type="button"
|
||||
onClick={cancelArmedSend}
|
||||
className="underline hover:opacity-80"
|
||||
>
|
||||
{t('conversation.attachments.cancelQueuedSend')}
|
||||
</button>
|
||||
</div>
|
||||
)}
|
||||
{sendArmed && sendReadiness.state === 'blocked' && (
|
||||
<div
|
||||
className="px-2 pb-1 text-xs text-[#B42318] sm:px-3"
|
||||
role="alert"
|
||||
>
|
||||
{t('conversation.attachments.sendBlockedByFailed', {
|
||||
names: sendReadiness.failedNames.join(', '),
|
||||
})}
|
||||
</div>
|
||||
)}
|
||||
{!sendArmed && completedAttachmentCount > 0 && (
|
||||
<div className="text-muted-foreground px-2 pb-1 text-xs sm:px-3">
|
||||
{t('conversation.attachments.attachedCount', {
|
||||
count: completedAttachmentCount,
|
||||
})}
|
||||
</div>
|
||||
)}
|
||||
{voiceError && (
|
||||
<div className="px-2 pb-1 text-xs text-[#B42318] sm:px-3">
|
||||
{voiceError}
|
||||
|
||||
@@ -0,0 +1,163 @@
|
||||
import { act } from 'react';
|
||||
import { createRoot, type Root } from 'react-dom/client';
|
||||
|
||||
import type { Attachment } from '../../upload/uploadSlice';
|
||||
import { getSendReadiness, useArmedSend } from './armedSend';
|
||||
|
||||
Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true });
|
||||
|
||||
const att = (over: Partial<Attachment> = {}): Attachment => ({
|
||||
id: 'a1',
|
||||
fileName: 'report.pdf',
|
||||
progress: 100,
|
||||
status: 'completed',
|
||||
taskId: 't1',
|
||||
...over,
|
||||
});
|
||||
|
||||
describe('getSendReadiness', () => {
|
||||
it('is ready with no attachments', () => {
|
||||
expect(getSendReadiness([])).toEqual({ state: 'ready' });
|
||||
});
|
||||
|
||||
it('is ready when every attachment is completed', () => {
|
||||
expect(getSendReadiness([att(), att({ id: 'a2' })])).toEqual({
|
||||
state: 'ready',
|
||||
});
|
||||
});
|
||||
|
||||
it('waits while any attachment is uploading or processing', () => {
|
||||
expect(
|
||||
getSendReadiness([
|
||||
att(),
|
||||
att({ id: 'a2', status: 'uploading', progress: 5 }),
|
||||
att({ id: 'a3', status: 'processing', progress: 40 }),
|
||||
]),
|
||||
).toEqual({ state: 'waiting', pendingCount: 2 });
|
||||
});
|
||||
|
||||
it('blocks on a failed attachment, listing its name', () => {
|
||||
expect(
|
||||
getSendReadiness([
|
||||
att(),
|
||||
att({ id: 'a2', status: 'failed', fileName: 'broken.pdf' }),
|
||||
]),
|
||||
).toEqual({ state: 'blocked', failedNames: ['broken.pdf'] });
|
||||
});
|
||||
|
||||
it('failed takes precedence over pending', () => {
|
||||
expect(
|
||||
getSendReadiness([
|
||||
att({ status: 'processing' }),
|
||||
att({ id: 'a2', status: 'failed', fileName: 'broken.pdf' }),
|
||||
]),
|
||||
).toEqual({ state: 'blocked', failedNames: ['broken.pdf'] });
|
||||
});
|
||||
});
|
||||
|
||||
type HookApi = ReturnType<typeof useArmedSend>;
|
||||
|
||||
function Host({
|
||||
attachments,
|
||||
onFlush,
|
||||
api,
|
||||
}: {
|
||||
attachments: Attachment[];
|
||||
onFlush: () => void;
|
||||
api: { current: HookApi | null };
|
||||
}) {
|
||||
const hook = useArmedSend({ attachments, onFlush });
|
||||
api.current = hook;
|
||||
return null;
|
||||
}
|
||||
|
||||
describe('useArmedSend', () => {
|
||||
let container: HTMLDivElement;
|
||||
let root: Root;
|
||||
let api: { current: HookApi | null };
|
||||
let onFlush: ReturnType<typeof vi.fn<() => void>>;
|
||||
|
||||
beforeEach(() => {
|
||||
container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
root = createRoot(container);
|
||||
api = { current: null };
|
||||
onFlush = vi.fn<() => void>();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await act(async () => root.unmount());
|
||||
container.remove();
|
||||
});
|
||||
|
||||
const render = async (attachments: Attachment[]) => {
|
||||
await act(async () => {
|
||||
root.render(
|
||||
<Host attachments={attachments} onFlush={onFlush} api={api} />,
|
||||
);
|
||||
});
|
||||
};
|
||||
|
||||
it('arms without flushing while attachments are pending', async () => {
|
||||
await render([att({ status: 'processing', progress: 30 })]);
|
||||
await act(async () => api.current!.arm());
|
||||
|
||||
expect(api.current!.armed).toBe(true);
|
||||
expect(api.current!.readiness.state).toBe('waiting');
|
||||
expect(onFlush).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('auto-flushes exactly once when all attachments complete', async () => {
|
||||
await render([att({ status: 'processing', progress: 30 })]);
|
||||
await act(async () => api.current!.arm());
|
||||
|
||||
await render([att()]);
|
||||
expect(onFlush).toHaveBeenCalledTimes(1);
|
||||
expect(api.current!.armed).toBe(false);
|
||||
|
||||
// A later re-render with the same resolved attachments must not re-fire.
|
||||
await render([att()]);
|
||||
expect(onFlush).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('holds the flush while a file is failed and resumes when it is removed', async () => {
|
||||
await render([att({ status: 'processing' })]);
|
||||
await act(async () => api.current!.arm());
|
||||
|
||||
await render([att({ status: 'failed', fileName: 'broken.pdf' })]);
|
||||
expect(onFlush).not.toHaveBeenCalled();
|
||||
expect(api.current!.armed).toBe(true);
|
||||
expect(api.current!.readiness).toEqual({
|
||||
state: 'blocked',
|
||||
failedNames: ['broken.pdf'],
|
||||
});
|
||||
|
||||
await render([]);
|
||||
expect(onFlush).toHaveBeenCalledTimes(1);
|
||||
expect(api.current!.armed).toBe(false);
|
||||
});
|
||||
|
||||
it('cancel disarms and prevents the auto-flush', async () => {
|
||||
await render([att({ status: 'uploading', progress: 10 })]);
|
||||
await act(async () => api.current!.arm());
|
||||
await act(async () => api.current!.cancel());
|
||||
|
||||
expect(api.current!.armed).toBe(false);
|
||||
|
||||
await render([att()]);
|
||||
expect(onFlush).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('flush uses the latest onFlush callback', async () => {
|
||||
await render([att({ status: 'processing' })]);
|
||||
await act(async () => api.current!.arm());
|
||||
|
||||
const lateFlush = vi.fn();
|
||||
await act(async () => {
|
||||
root.render(<Host attachments={[att()]} onFlush={lateFlush} api={api} />);
|
||||
});
|
||||
|
||||
expect(onFlush).not.toHaveBeenCalled();
|
||||
expect(lateFlush).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,56 @@
|
||||
import { useCallback, useEffect, useRef, useState } from 'react';
|
||||
|
||||
import type { Attachment } from '../../upload/uploadSlice';
|
||||
|
||||
export type SendReadiness =
|
||||
| { state: 'ready' }
|
||||
| { state: 'waiting'; pendingCount: number }
|
||||
| { state: 'blocked'; failedNames: string[] };
|
||||
|
||||
export function getSendReadiness(attachments: Attachment[]): SendReadiness {
|
||||
const failedNames = attachments
|
||||
.filter((a) => a.status === 'failed')
|
||||
.map((a) => a.fileName);
|
||||
if (failedNames.length > 0) return { state: 'blocked', failedNames };
|
||||
|
||||
const pendingCount = attachments.filter(
|
||||
(a) => a.status === 'uploading' || a.status === 'processing',
|
||||
).length;
|
||||
if (pendingCount > 0) return { state: 'waiting', pendingCount };
|
||||
|
||||
return { state: 'ready' };
|
||||
}
|
||||
|
||||
export function useArmedSend({
|
||||
attachments,
|
||||
onFlush,
|
||||
}: {
|
||||
attachments: Attachment[];
|
||||
onFlush: () => void;
|
||||
}) {
|
||||
const [armed, setArmed] = useState(false);
|
||||
// Latest-closure ref so the flush submits the current composer value,
|
||||
// not the one captured when the send was armed.
|
||||
const flushRef = useRef(onFlush);
|
||||
flushRef.current = onFlush;
|
||||
const flushedRef = useRef(false);
|
||||
|
||||
const readiness = getSendReadiness(attachments);
|
||||
|
||||
useEffect(() => {
|
||||
if (!armed) {
|
||||
flushedRef.current = false;
|
||||
return;
|
||||
}
|
||||
if (readiness.state === 'ready' && !flushedRef.current) {
|
||||
flushedRef.current = true;
|
||||
setArmed(false);
|
||||
flushRef.current();
|
||||
}
|
||||
}, [armed, readiness.state]);
|
||||
|
||||
const arm = useCallback(() => setArmed(true), []);
|
||||
const cancel = useCallback(() => setArmed(false), []);
|
||||
|
||||
return { armed, readiness, arm, cancel };
|
||||
}
|
||||
@@ -7,6 +7,7 @@ import userService from '../api/services/userService';
|
||||
import SharedAgentCard from '../agents/SharedAgentCard';
|
||||
import { Agent } from '../agents/types';
|
||||
import ArtifactSidebar from '../components/ArtifactSidebar';
|
||||
import ErrorBoundary from '../components/ErrorBoundary';
|
||||
import MessageInput from '../components/MessageInput';
|
||||
import { useMediaQuery } from '../hooks';
|
||||
import {
|
||||
@@ -31,7 +32,10 @@ import {
|
||||
submitToolActions,
|
||||
updateQuery,
|
||||
} from './conversationSlice';
|
||||
import { selectCompletedAttachments } from '../upload/uploadSlice';
|
||||
import {
|
||||
clearAttachments,
|
||||
selectCompletedAttachments,
|
||||
} from '../upload/uploadSlice';
|
||||
|
||||
export default function Conversation() {
|
||||
const { t } = useTranslation();
|
||||
@@ -165,8 +169,16 @@ export default function Conversation() {
|
||||
}, [conversationId, status]);
|
||||
|
||||
const handleFetchAnswer = useCallback(
|
||||
({ question, index }: { question: string; index?: number }) => {
|
||||
dispatch(fetchAnswer({ question, indx: index }));
|
||||
({
|
||||
question,
|
||||
index,
|
||||
attachmentIds,
|
||||
}: {
|
||||
question: string;
|
||||
index?: number;
|
||||
attachmentIds?: string[];
|
||||
}) => {
|
||||
dispatch(fetchAnswer({ question, indx: index, attachmentIds }));
|
||||
},
|
||||
[dispatch, selectedAgent],
|
||||
);
|
||||
@@ -184,11 +196,12 @@ export default function Conversation() {
|
||||
const trimmedQuestion = question.trim();
|
||||
if (trimmedQuestion === '') return;
|
||||
|
||||
const filesAttached = completedAttachments
|
||||
.filter((a) => a.id)
|
||||
.map((a) => ({ id: a.id as string, fileName: a.fileName }));
|
||||
|
||||
if (index !== undefined) {
|
||||
// Retry/edit of an existing turn: re-send the ids bound to that
|
||||
// row — the composer slice was consumed by the original send.
|
||||
const rowAttachmentIds = (queries[index]?.attachments ?? []).map(
|
||||
(a) => a.id,
|
||||
);
|
||||
dispatch(
|
||||
resendQuery({
|
||||
index,
|
||||
@@ -196,8 +209,16 @@ export default function Conversation() {
|
||||
keepIdempotencyKey: isRetry,
|
||||
}),
|
||||
);
|
||||
handleFetchAnswer({ question: trimmedQuestion, index });
|
||||
handleFetchAnswer({
|
||||
question: trimmedQuestion,
|
||||
index,
|
||||
attachmentIds: rowAttachmentIds,
|
||||
});
|
||||
} else {
|
||||
const filesAttached = completedAttachments
|
||||
.filter((a) => a.id)
|
||||
.map((a) => ({ id: a.id as string, fileName: a.fileName }));
|
||||
|
||||
if (!isRetry)
|
||||
dispatch(
|
||||
addQuery({
|
||||
@@ -205,10 +226,17 @@ export default function Conversation() {
|
||||
attachments: filesAttached,
|
||||
}),
|
||||
);
|
||||
handleFetchAnswer({ question: trimmedQuestion, index });
|
||||
// One source of truth: the ids on the wire are exactly the ids
|
||||
// the optimistic row displays.
|
||||
handleFetchAnswer({
|
||||
question: trimmedQuestion,
|
||||
index,
|
||||
attachmentIds: filesAttached.map((f) => f.id),
|
||||
});
|
||||
if (filesAttached.length > 0) dispatch(clearAttachments());
|
||||
}
|
||||
},
|
||||
[dispatch, handleFetchAnswer, completedAttachments],
|
||||
[dispatch, handleFetchAnswer, completedAttachments, queries],
|
||||
);
|
||||
|
||||
const handleFeedback = (query: Query, feedback: FEEDBACK, index: number) => {
|
||||
@@ -336,38 +364,41 @@ export default function Conversation() {
|
||||
}`}
|
||||
>
|
||||
<div className="relative min-h-0 flex-1">
|
||||
<ConversationMessages
|
||||
key={conversationMountKey}
|
||||
handleQuestion={handleQuestion}
|
||||
handleQuestionSubmission={handleQuestionSubmission}
|
||||
handleFeedback={handleFeedback}
|
||||
queries={queries}
|
||||
status={status}
|
||||
showHeroOnEmpty={selectedAgent ? false : true}
|
||||
onOpenArtifact={handleOpenArtifact}
|
||||
onToolAction={handleToolAction}
|
||||
isSplitView={isSplitArtifactOpen}
|
||||
agentId={selectedAgent?.id}
|
||||
headerContent={
|
||||
selectedAgent ? (
|
||||
<div className="flex w-full items-center justify-center py-4">
|
||||
<SharedAgentCard
|
||||
agent={selectedAgent}
|
||||
onEdit={
|
||||
selectedAgent.id
|
||||
? () =>
|
||||
navigate(
|
||||
selectedAgent.agent_type === 'workflow'
|
||||
? `/agents/workflow/edit/${selectedAgent.id}`
|
||||
: `/agents/edit/${selectedAgent.id}`,
|
||||
)
|
||||
: undefined
|
||||
}
|
||||
/>
|
||||
</div>
|
||||
) : undefined
|
||||
}
|
||||
/>
|
||||
{/* A render crash in the message list must leave the composer
|
||||
usable; the boundary resets on conversation switch. */}
|
||||
<ErrorBoundary key={conversationMountKey}>
|
||||
<ConversationMessages
|
||||
handleQuestion={handleQuestion}
|
||||
handleQuestionSubmission={handleQuestionSubmission}
|
||||
handleFeedback={handleFeedback}
|
||||
queries={queries}
|
||||
status={status}
|
||||
showHeroOnEmpty={selectedAgent ? false : true}
|
||||
onOpenArtifact={handleOpenArtifact}
|
||||
onToolAction={handleToolAction}
|
||||
isSplitView={isSplitArtifactOpen}
|
||||
agentId={selectedAgent?.id}
|
||||
headerContent={
|
||||
selectedAgent ? (
|
||||
<div className="flex w-full items-center justify-center py-4">
|
||||
<SharedAgentCard
|
||||
agent={selectedAgent}
|
||||
onEdit={
|
||||
selectedAgent.id
|
||||
? () =>
|
||||
navigate(
|
||||
selectedAgent.agent_type === 'workflow'
|
||||
? `/agents/workflow/edit/${selectedAgent.id}`
|
||||
: `/agents/edit/${selectedAgent.id}`,
|
||||
)
|
||||
: undefined
|
||||
}
|
||||
/>
|
||||
</div>
|
||||
) : undefined
|
||||
}
|
||||
/>
|
||||
</ErrorBoundary>
|
||||
<div
|
||||
className={`from-background pointer-events-none absolute bottom-0 left-1/2 h-6 w-full -translate-x-1/2 rounded-t-2xl bg-linear-to-t to-transparent bg-clip-content px-2 ${
|
||||
isSplitArtifactOpen
|
||||
|
||||
@@ -1,8 +1,22 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
vi.mock('./conversationHandlers', () => ({
|
||||
handleFetchAnswer: vi.fn().mockResolvedValue(null),
|
||||
handleFetchAnswerSteaming: vi.fn().mockResolvedValue(undefined),
|
||||
handleSubmitToolActions: vi.fn(),
|
||||
handleV1ChatCompletionStreaming: vi.fn(),
|
||||
}));
|
||||
|
||||
import { configureStore } from '@reduxjs/toolkit';
|
||||
|
||||
import uploadReducer, { addAttachment } from '../upload/uploadSlice';
|
||||
import { handleFetchAnswer } from './conversationHandlers';
|
||||
import reducer, {
|
||||
addQuery,
|
||||
applyMessageTail,
|
||||
fetchAnswer,
|
||||
raiseNotice,
|
||||
resendQuery,
|
||||
setConversation,
|
||||
} from './conversationSlice';
|
||||
|
||||
@@ -182,3 +196,135 @@ describe('raiseNotice — non-fatal notice', () => {
|
||||
expect(next.queries[0].notice).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
const completedAtt = {
|
||||
id: 'srv-1',
|
||||
fileName: 'a.pdf',
|
||||
progress: 100,
|
||||
status: 'completed' as const,
|
||||
taskId: 't1',
|
||||
};
|
||||
const processingAtt = {
|
||||
id: 'c-2',
|
||||
fileName: 'b.pdf',
|
||||
progress: 40,
|
||||
status: 'processing' as const,
|
||||
taskId: 't2',
|
||||
};
|
||||
|
||||
const preferenceStub = {
|
||||
token: 'tok',
|
||||
selectedDocs: [],
|
||||
prompt: { id: 'default' },
|
||||
chunks: '2',
|
||||
selectedAgent: null,
|
||||
selectedModel: null,
|
||||
};
|
||||
|
||||
const makeStore = () =>
|
||||
configureStore({
|
||||
reducer: {
|
||||
conversation: reducer,
|
||||
upload: uploadReducer,
|
||||
preference: () => preferenceStub,
|
||||
},
|
||||
});
|
||||
|
||||
describe('fetchAnswer — attachment ids on the wire', () => {
|
||||
beforeEach(() => {
|
||||
vi.mocked(handleFetchAnswer)
|
||||
.mockClear()
|
||||
.mockResolvedValue(null as never);
|
||||
});
|
||||
|
||||
it('sends explicit attachmentIds and leaves composer attachments untouched', async () => {
|
||||
const store = makeStore();
|
||||
store.dispatch(addQuery({ prompt: 'q' }));
|
||||
store.dispatch(addAttachment(completedAtt));
|
||||
|
||||
await store.dispatch(
|
||||
fetchAnswer({
|
||||
question: 'q',
|
||||
indx: 0,
|
||||
attachmentIds: ['row-1', 'row-2'],
|
||||
}),
|
||||
);
|
||||
|
||||
expect(vi.mocked(handleFetchAnswer).mock.calls[0][8]).toEqual([
|
||||
'row-1',
|
||||
'row-2',
|
||||
]);
|
||||
// Explicit ids (e.g. a retry of an existing turn) must not consume
|
||||
// whatever the composer currently holds.
|
||||
expect(store.getState().upload.attachments).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('falls back to completed composer uploads and clears them when no ids are passed', async () => {
|
||||
const store = makeStore();
|
||||
store.dispatch(addQuery({ prompt: 'q' }));
|
||||
store.dispatch(addAttachment(completedAtt));
|
||||
store.dispatch(addAttachment(processingAtt));
|
||||
|
||||
await store.dispatch(fetchAnswer({ question: 'q', indx: 0 }));
|
||||
|
||||
expect(vi.mocked(handleFetchAnswer).mock.calls[0][8]).toEqual(['srv-1']);
|
||||
// clearAttachments keeps in-flight rows.
|
||||
expect(store.getState().upload.attachments.map((a) => a.id)).toEqual([
|
||||
'c-2',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('fetchAnswer.rejected', () => {
|
||||
it('writes the error to the retried row, not the last row', () => {
|
||||
let state = reducer(
|
||||
undefined,
|
||||
setConversation([{ prompt: 'first' }, { prompt: 'second' }]),
|
||||
);
|
||||
state = reducer(
|
||||
state,
|
||||
fetchAnswer.rejected(new Error('boom'), 'req-1', {
|
||||
question: 'first',
|
||||
indx: 0,
|
||||
}),
|
||||
);
|
||||
expect(state.status).toBe('failed');
|
||||
expect(state.queries[0].error).toBe('Something went wrong');
|
||||
expect(state.queries[1].error).toBeUndefined();
|
||||
});
|
||||
|
||||
it('defaults to the last row when no index is given', () => {
|
||||
let state = reducer(
|
||||
undefined,
|
||||
setConversation([{ prompt: 'first' }, { prompt: 'second' }]),
|
||||
);
|
||||
state = reducer(
|
||||
state,
|
||||
fetchAnswer.rejected(new Error('boom'), 'req-1', { question: 'second' }),
|
||||
);
|
||||
expect(state.queries[0].error).toBeUndefined();
|
||||
expect(state.queries[1].error).toBe('Something went wrong');
|
||||
});
|
||||
});
|
||||
|
||||
describe('resendQuery', () => {
|
||||
it('preserves the attachments bound to the turn', () => {
|
||||
let state = reducer(
|
||||
undefined,
|
||||
setConversation([
|
||||
{
|
||||
prompt: 'p',
|
||||
response: 'r',
|
||||
error: 'e',
|
||||
attachments: [{ id: 'x', fileName: 'f.pdf' }],
|
||||
},
|
||||
]),
|
||||
);
|
||||
state = reducer(state, resendQuery({ index: 0, prompt: 'p2' }));
|
||||
expect(state.queries[0].attachments).toEqual([
|
||||
{ id: 'x', fileName: 'f.pdf' },
|
||||
]);
|
||||
expect(state.queries[0].response).toBeUndefined();
|
||||
expect(state.queries[0].error).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -160,20 +160,29 @@ export const loadConversation = createAsyncThunk<
|
||||
|
||||
export const fetchAnswer = createAsyncThunk<
|
||||
Answer,
|
||||
{ question: string; indx?: number }
|
||||
>('fetchAnswer', async ({ question, indx }, { dispatch, getState }) => {
|
||||
{ question: string; indx?: number; attachmentIds?: string[] }
|
||||
>('fetchAnswer', async (arg, { dispatch, getState }) => {
|
||||
const { question, indx } = arg;
|
||||
if (abortController) abortController.abort();
|
||||
abortController = new AbortController();
|
||||
const { signal } = abortController;
|
||||
|
||||
let isSourceUpdated = false;
|
||||
const state = getState() as RootState;
|
||||
const attachmentIds = selectCompletedAttachments(state)
|
||||
.filter((a) => a.id)
|
||||
.map((a) => a.id) as string[];
|
||||
|
||||
if (attachmentIds.length > 0) {
|
||||
dispatch(clearAttachments());
|
||||
// Explicit ids (new sends and retries pass the ids bound to the turn)
|
||||
// keep the wire payload in sync with what the query row displays.
|
||||
// The fallback reads the composer state for callers that predate the
|
||||
// argument and consumes it, which loses the ids on a later retry.
|
||||
let attachmentIds: string[];
|
||||
if (arg.attachmentIds !== undefined) {
|
||||
attachmentIds = arg.attachmentIds;
|
||||
} else {
|
||||
attachmentIds = selectCompletedAttachments(state)
|
||||
.filter((a) => a.id)
|
||||
.map((a) => a.id) as string[];
|
||||
if (attachmentIds.length > 0) {
|
||||
dispatch(clearAttachments());
|
||||
}
|
||||
}
|
||||
|
||||
// Mutable so the SSE handler can adopt a server-assigned id and
|
||||
@@ -1147,8 +1156,14 @@ export const conversationSlice = createSlice({
|
||||
}
|
||||
state.status = 'failed';
|
||||
if (state.queries.length > 0) {
|
||||
state.queries[state.queries.length - 1].error =
|
||||
'Something went wrong';
|
||||
// Indexed sends (edit/retry) must fail on their own row, not
|
||||
// whatever happens to be last.
|
||||
const indx = action.meta.arg?.indx;
|
||||
const targetIndex =
|
||||
indx !== undefined && indx >= 0 && indx < state.queries.length
|
||||
? indx
|
||||
: state.queries.length - 1;
|
||||
state.queries[targetIndex].error = 'Something went wrong';
|
||||
}
|
||||
});
|
||||
},
|
||||
|
||||
@@ -8,6 +8,10 @@
|
||||
"sourceDocs": "Source",
|
||||
"none": "None",
|
||||
"cancel": "Cancel",
|
||||
"errorBoundary": {
|
||||
"message": "Something went wrong displaying this content.",
|
||||
"tryAgain": "Try again"
|
||||
},
|
||||
"help": "Help",
|
||||
"emailUs": "Email us",
|
||||
"documentation": "Documentation",
|
||||
@@ -1090,7 +1094,13 @@
|
||||
},
|
||||
"attachments": {
|
||||
"attach": "Attach",
|
||||
"remove": "Remove attachment"
|
||||
"remove": "Remove attachment",
|
||||
"waitingToSend_one": "Will send when {{count}} file finishes processing…",
|
||||
"waitingToSend_other": "Will send when {{count}} files finish processing…",
|
||||
"cancelQueuedSend": "Cancel",
|
||||
"sendBlockedByFailed": "{{names}} couldn't be processed — remove to send",
|
||||
"attachedCount_one": "Attached: {{count}} file",
|
||||
"attachedCount_other": "Attached: {{count}} files"
|
||||
},
|
||||
"retry": "Retry",
|
||||
"reasoning": "Reasoning",
|
||||
|
||||
@@ -0,0 +1,198 @@
|
||||
/**
|
||||
* Composer resilience: the composer must never silently discard a file
|
||||
* or a send.
|
||||
*
|
||||
* Two silent-failure paths pinned here:
|
||||
*
|
||||
* 1. Stream POST failure. A failed /stream request must surface a
|
||||
* visible error bubble with a working Retry — never a send that
|
||||
* evaporates. (fetchAnswer.rejected → per-row error → Retry.)
|
||||
*
|
||||
* 2. Send while an attachment is still uploading/parsing. The composer
|
||||
* "arms" the send (visible "Will send when … " banner), holds the
|
||||
* message, and auto-flushes once the attachment completes — so the
|
||||
* file arrives bound to the turn (`conversation_messages.attachments[]`)
|
||||
* instead of being silently dropped from the payload.
|
||||
*
|
||||
* UI-driven on purpose: the surface under test IS the composer. The
|
||||
* attachments spec avoids the file picker for API-direct upload tests;
|
||||
* here `setInputFiles` on the dropzone input is the point.
|
||||
*/
|
||||
|
||||
import { dirname, resolve } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
||||
import * as playwright from '@playwright/test';
|
||||
const { expect, test } = playwright;
|
||||
|
||||
import type { Page } from '@playwright/test';
|
||||
|
||||
import { newUserContext } from '../../helpers/auth.js';
|
||||
import { pg } from '../../helpers/db.js';
|
||||
import { resetDb } from '../../helpers/reset.js';
|
||||
|
||||
const HERE = dirname(fileURLToPath(import.meta.url));
|
||||
const FIXTURES_DIR = resolve(HERE, '..', '..', 'fixtures', 'docs');
|
||||
const SMALL_FIXTURE_PATH = resolve(FIXTURES_DIR, 'notes.txt');
|
||||
|
||||
async function openComposer(page: Page): Promise<void> {
|
||||
await page.goto('/');
|
||||
await expect(page.locator('#message-input')).toBeVisible();
|
||||
}
|
||||
|
||||
test.describe('tier-a · composer resilience', () => {
|
||||
test.beforeEach(async () => {
|
||||
await resetDb();
|
||||
});
|
||||
|
||||
test('failed /stream surfaces a visible error and Retry re-sends successfully', async ({
|
||||
browser,
|
||||
}) => {
|
||||
const { context, sub } = await newUserContext(browser);
|
||||
try {
|
||||
const page = await context.newPage();
|
||||
await openComposer(page);
|
||||
|
||||
// Force the stream POST to die at the network layer — the shape of
|
||||
// an outage the frontend cannot see an HTTP status for.
|
||||
await page.route('**/stream', (route) => route.abort('failed'));
|
||||
|
||||
const textarea = page.locator('#message-input');
|
||||
await textarea.fill('resilience probe — e2e-composer-retry');
|
||||
await textarea.press('Enter');
|
||||
|
||||
// Visible failure, not a silent drop: the error bubble and a
|
||||
// usable Retry control.
|
||||
await expect(page.getByText('Something went wrong')).toBeVisible({
|
||||
timeout: 15_000,
|
||||
});
|
||||
const retryButton = page.getByRole('button', { name: /retry/i });
|
||||
await expect(retryButton).toBeEnabled();
|
||||
|
||||
// Network heals; Retry must complete the SAME turn.
|
||||
await page.unroute('**/stream');
|
||||
const streamDone = page.waitForResponse(
|
||||
(r) => r.url().includes('/stream') && r.request().method() === 'POST',
|
||||
{ timeout: 45_000 },
|
||||
);
|
||||
await retryButton.click();
|
||||
const streamRes = await streamDone;
|
||||
expect(streamRes.status()).toBe(200);
|
||||
|
||||
await expect(page.getByText('Something went wrong')).toBeHidden({
|
||||
timeout: 15_000,
|
||||
});
|
||||
|
||||
// The turn landed: one message row, prompt AND response populated.
|
||||
const { rows } = await pg.query<{
|
||||
prompt: string | null;
|
||||
response: string | null;
|
||||
}>(
|
||||
`SELECT cm.prompt, cm.response
|
||||
FROM conversation_messages cm
|
||||
JOIN conversations c ON c.id = cm.conversation_id
|
||||
WHERE c.user_id = $1`,
|
||||
[sub],
|
||||
);
|
||||
expect(rows).toHaveLength(1);
|
||||
expect(rows[0].prompt).toBe('resilience probe — e2e-composer-retry');
|
||||
expect(rows[0].response).toBeTruthy();
|
||||
} finally {
|
||||
await context.close();
|
||||
}
|
||||
});
|
||||
|
||||
test('send while a file is still processing arms, waits, and flushes with the file bound to the turn', async ({
|
||||
browser,
|
||||
}) => {
|
||||
const { context, sub } = await newUserContext(browser);
|
||||
try {
|
||||
const page = await context.newPage();
|
||||
await openComposer(page);
|
||||
|
||||
// Track /stream POSTs so we can prove the send was HELD.
|
||||
const streamPosts: string[] = [];
|
||||
page.on('request', (req) => {
|
||||
if (req.url().includes('/stream') && req.method() === 'POST') {
|
||||
streamPosts.push(req.postData() ?? '');
|
||||
}
|
||||
});
|
||||
|
||||
// Hold the upload response long enough to press Enter while the
|
||||
// attachment is still visibly 'uploading'.
|
||||
await page.route('**/api/store_attachment', async (route) => {
|
||||
await new Promise((r) => setTimeout(r, 2_000));
|
||||
await route.continue();
|
||||
});
|
||||
|
||||
// The dropzone input is the first file input MessageInput renders
|
||||
// (the second is the audio-capture input).
|
||||
await page
|
||||
.locator('input[type="file"]')
|
||||
.first()
|
||||
.setInputFiles(SMALL_FIXTURE_PATH);
|
||||
|
||||
const textarea = page.locator('#message-input');
|
||||
await textarea.fill('what is in the attached file? e2e-armed-send');
|
||||
await textarea.press('Enter');
|
||||
|
||||
// The send is armed, not fired: banner visible, no /stream yet,
|
||||
// and the prompt is still in the composer.
|
||||
await expect(
|
||||
page.getByText(/will send when 1 file finishes processing/i),
|
||||
).toBeVisible({ timeout: 5_000 });
|
||||
expect(streamPosts).toHaveLength(0);
|
||||
await expect(textarea).toHaveValue(
|
||||
'what is in the attached file? e2e-armed-send',
|
||||
);
|
||||
|
||||
// Upload completes → Celery parses → attachment.completed lands via
|
||||
// SSE → the armed send auto-flushes. notes.txt parses in seconds.
|
||||
const streamDone = page.waitForResponse(
|
||||
(r) => r.url().includes('/stream') && r.request().method() === 'POST',
|
||||
{ timeout: 60_000 },
|
||||
);
|
||||
const streamRes = await streamDone;
|
||||
expect(streamRes.status()).toBe(200);
|
||||
|
||||
// The flushed payload carried exactly one attachment id.
|
||||
expect(streamPosts).toHaveLength(1);
|
||||
const payload = JSON.parse(streamPosts[0]) as {
|
||||
attachments?: string[];
|
||||
};
|
||||
expect(payload.attachments).toHaveLength(1);
|
||||
|
||||
// Banner cleared, composer emptied by the flush.
|
||||
await expect(
|
||||
page.getByText(/will send when/i),
|
||||
).toBeHidden({ timeout: 15_000 });
|
||||
await expect(textarea).toHaveValue('');
|
||||
|
||||
// The invariant that motivated all of this: the file is BOUND to
|
||||
// the turn — conversation_messages.attachments[] holds one PG PK
|
||||
// that resolves to this user's attachments row.
|
||||
const { rows } = await pg.query<{
|
||||
prompt: string | null;
|
||||
attachment_count: number | string | null;
|
||||
resolved: number | string | null;
|
||||
}>(
|
||||
`SELECT cm.prompt,
|
||||
COALESCE(array_length(cm.attachments, 1), 0) AS attachment_count,
|
||||
(SELECT count(*) FROM attachments a
|
||||
WHERE a.id = ANY(cm.attachments)) AS resolved
|
||||
FROM conversation_messages cm
|
||||
JOIN conversations c ON c.id = cm.conversation_id
|
||||
WHERE c.user_id = $1`,
|
||||
[sub],
|
||||
);
|
||||
expect(rows).toHaveLength(1);
|
||||
expect(rows[0].prompt).toBe(
|
||||
'what is in the attached file? e2e-armed-send',
|
||||
);
|
||||
expect(Number(rows[0].attachment_count)).toBe(1);
|
||||
expect(Number(rows[0].resolved)).toBe(1);
|
||||
} finally {
|
||||
await context.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user