Refine chat tool status dedupe (#786)
Co-authored-by: zuolingxuan <zuolingxuan@bytedance.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
32d14b8cf9
commit
25b13ab912
@@ -95,6 +95,16 @@ status: completed successfully`,
|
||||
{
|
||||
role: 'assistant',
|
||||
content: [{ type: 'text', text: '我让 coder 分析完了,下面是结论。' }],
|
||||
_attachedFiles: [
|
||||
{
|
||||
fileName: 'CHECKLIST.md',
|
||||
mimeType: 'text/markdown',
|
||||
fileSize: 433,
|
||||
preview: null,
|
||||
filePath: '/Users/bytedance/.openclaw/workspace/CHECKLIST.md',
|
||||
source: 'tool-result',
|
||||
},
|
||||
],
|
||||
timestamp: Date.now(),
|
||||
},
|
||||
];
|
||||
@@ -203,7 +213,13 @@ test.describe('ClawX chat execution graph', () => {
|
||||
});
|
||||
|
||||
const page = await getStableWindow(app);
|
||||
await page.reload();
|
||||
try {
|
||||
await page.reload();
|
||||
} catch (error) {
|
||||
if (!String(error).includes('ERR_FILE_NOT_FOUND')) {
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
await expect(page.getByTestId('main-layout')).toBeVisible();
|
||||
await expect(page.getByTestId('chat-execution-graph')).toBeVisible({ timeout: 30_000 });
|
||||
await expect(
|
||||
@@ -214,6 +230,7 @@ test.describe('ClawX chat execution graph', () => {
|
||||
page.locator('[data-testid="chat-execution-graph"] [data-testid="chat-execution-step"]').getByText('exec', { exact: true }),
|
||||
).toBeVisible();
|
||||
await expect(page.locator('[data-testid="chat-execution-graph"]').getByText('我让 coder 去拆 ~/Velaria 当前未提交改动的核心块了,等它回来我直接给你结论。')).toBeVisible();
|
||||
await expect(page.getByText('CHECKLIST.md')).toHaveCount(0);
|
||||
} finally {
|
||||
await closeElectronApp(app);
|
||||
}
|
||||
|
||||
33
tests/unit/chat-message.test.tsx
Normal file
33
tests/unit/chat-message.test.tsx
Normal file
@@ -0,0 +1,33 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import { ChatMessage } from '@/pages/Chat/ChatMessage';
|
||||
import type { RawMessage } from '@/stores/chat';
|
||||
|
||||
describe('ChatMessage attachment dedupe', () => {
|
||||
it('keeps attachment-only assistant replies visible even when process attachments are suppressed', () => {
|
||||
const message: RawMessage = {
|
||||
role: 'assistant',
|
||||
content: [],
|
||||
_attachedFiles: [
|
||||
{
|
||||
fileName: 'artifact.png',
|
||||
mimeType: 'image/png',
|
||||
fileSize: 0,
|
||||
preview: '/tmp/artifact.png',
|
||||
filePath: '/tmp/artifact.png',
|
||||
source: 'tool-result',
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
render(
|
||||
<ChatMessage
|
||||
message={message}
|
||||
showThinking={false}
|
||||
suppressProcessAttachments
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByAltText('artifact.png')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
@@ -11,13 +11,14 @@ const getToolCallFilePath = vi.fn(() => undefined);
|
||||
const hasErrorRecoveryTimer = vi.fn(() => false);
|
||||
const hasNonToolAssistantContent = vi.fn(() => true);
|
||||
const isToolOnlyMessage = vi.fn(() => false);
|
||||
const isToolResultRole = vi.fn((role: unknown) => role === 'toolresult');
|
||||
const makeAttachedFile = vi.fn((ref: { filePath: string; mimeType: string }) => ({
|
||||
const isToolResultRole = vi.fn((role: unknown) => role === 'toolresult' || role === 'toolResult' || role === 'tool_result');
|
||||
const makeAttachedFile = vi.fn((ref: { filePath: string; mimeType: string }, source?: 'user-upload' | 'tool-result' | 'message-ref') => ({
|
||||
fileName: ref.filePath.split('/').pop() || 'file',
|
||||
mimeType: ref.mimeType,
|
||||
fileSize: 0,
|
||||
preview: null,
|
||||
filePath: ref.filePath,
|
||||
source,
|
||||
}));
|
||||
const setErrorRecoveryTimer = vi.fn();
|
||||
const upsertToolStatuses = vi.fn((_current, updates) => updates);
|
||||
@@ -128,6 +129,51 @@ describe('chat runtime event handlers', () => {
|
||||
expect(h.read().loadHistory).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('marks tool-result attachments before appending them to the final assistant reply', async () => {
|
||||
extractMediaRefs.mockReturnValue([{ filePath: '/tmp/CHECKLIST.md', mimeType: 'text/markdown' }]);
|
||||
getMessageText.mockReturnValue('[media attached: /tmp/CHECKLIST.md (text/markdown) | /tmp/CHECKLIST.md]');
|
||||
hasNonToolAssistantContent.mockReturnValue(true);
|
||||
|
||||
const { handleRuntimeEventState } = await import('@/stores/chat/runtime-event-handlers');
|
||||
const h = makeHarness({
|
||||
pendingToolImages: [],
|
||||
streamingMessage: {
|
||||
role: 'assistant',
|
||||
id: 'streaming-assistant',
|
||||
content: [{ type: 'tool_use', id: 'call-1', name: 'read', input: { filePath: '/tmp/CHECKLIST.md' } }],
|
||||
},
|
||||
});
|
||||
|
||||
handleRuntimeEventState(h.set as never, h.get as never, {
|
||||
message: {
|
||||
role: 'toolResult',
|
||||
toolCallId: 'call-1',
|
||||
toolName: 'read',
|
||||
content: [{ type: 'text', text: '[media attached: /tmp/CHECKLIST.md (text/markdown) | /tmp/CHECKLIST.md]' }],
|
||||
},
|
||||
}, 'final', 'run-4');
|
||||
|
||||
handleRuntimeEventState(h.set as never, h.get as never, {
|
||||
message: {
|
||||
role: 'assistant',
|
||||
id: 'final-assistant',
|
||||
content: [{ type: 'text', text: 'Done.' }],
|
||||
},
|
||||
}, 'final', 'run-4');
|
||||
|
||||
expect(h.read().messages).toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
id: 'final-assistant',
|
||||
_attachedFiles: [
|
||||
expect.objectContaining({
|
||||
filePath: '/tmp/CHECKLIST.md',
|
||||
source: 'tool-result',
|
||||
}),
|
||||
],
|
||||
}),
|
||||
]));
|
||||
});
|
||||
|
||||
it('handles error event and finalizes immediately when not sending', async () => {
|
||||
const { handleRuntimeEventState } = await import('@/stores/chat/runtime-event-handlers');
|
||||
const h = makeHarness({ sending: false, activeRunId: 'r1', lastUserMessageAt: 123 });
|
||||
@@ -203,4 +249,3 @@ describe('chat runtime event handlers', () => {
|
||||
expect(next.pendingToolImages).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -43,6 +43,129 @@ describe('deriveTaskSteps', () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it('keeps completed tool steps visible while a later tool is still streaming', () => {
|
||||
const steps = deriveTaskSteps({
|
||||
messages: [
|
||||
{
|
||||
role: 'assistant',
|
||||
id: 'assistant-history',
|
||||
content: [
|
||||
{ type: 'tool_use', id: 'tool-read', name: 'read', input: { filePath: '/tmp/a.md' } },
|
||||
],
|
||||
},
|
||||
],
|
||||
streamingMessage: {
|
||||
role: 'assistant',
|
||||
content: [
|
||||
{ type: 'tool_use', id: 'tool-grep', name: 'grep', input: { pattern: 'TODO' } },
|
||||
],
|
||||
},
|
||||
streamingTools: [
|
||||
{
|
||||
toolCallId: 'tool-grep',
|
||||
name: 'grep',
|
||||
status: 'running',
|
||||
updatedAt: Date.now(),
|
||||
summary: 'Scanning files',
|
||||
},
|
||||
],
|
||||
sending: true,
|
||||
pendingFinal: false,
|
||||
showThinking: false,
|
||||
});
|
||||
|
||||
expect(steps).toEqual([
|
||||
expect.objectContaining({
|
||||
id: 'tool-read',
|
||||
label: 'read',
|
||||
status: 'completed',
|
||||
kind: 'tool',
|
||||
}),
|
||||
expect.objectContaining({
|
||||
id: 'tool-grep',
|
||||
label: 'grep',
|
||||
status: 'running',
|
||||
kind: 'tool',
|
||||
}),
|
||||
]);
|
||||
});
|
||||
|
||||
it('upgrades a completed historical tool step when streaming status reports a later state', () => {
|
||||
const steps = deriveTaskSteps({
|
||||
messages: [
|
||||
{
|
||||
role: 'assistant',
|
||||
id: 'assistant-history',
|
||||
content: [
|
||||
{ type: 'tool_use', id: 'tool-read', name: 'read', input: { filePath: '/tmp/a.md' } },
|
||||
],
|
||||
},
|
||||
],
|
||||
streamingMessage: null,
|
||||
streamingTools: [
|
||||
{
|
||||
toolCallId: 'tool-read',
|
||||
name: 'read',
|
||||
status: 'error',
|
||||
updatedAt: Date.now(),
|
||||
summary: 'Permission denied',
|
||||
},
|
||||
],
|
||||
sending: true,
|
||||
pendingFinal: false,
|
||||
showThinking: false,
|
||||
});
|
||||
|
||||
expect(steps).toEqual([
|
||||
expect.objectContaining({
|
||||
id: 'tool-read',
|
||||
label: 'read',
|
||||
status: 'error',
|
||||
kind: 'tool',
|
||||
detail: 'Permission denied',
|
||||
}),
|
||||
]);
|
||||
});
|
||||
|
||||
it('keeps the newest running step when the execution graph exceeds the max length', () => {
|
||||
const messages: RawMessage[] = Array.from({ length: 9 }, (_, index) => ({
|
||||
role: 'assistant',
|
||||
id: `assistant-${index}`,
|
||||
content: [
|
||||
{ type: 'tool_use', id: `tool-${index}`, name: `read_${index}`, input: { filePath: `/tmp/${index}.md` } },
|
||||
],
|
||||
}));
|
||||
|
||||
const steps = deriveTaskSteps({
|
||||
messages,
|
||||
streamingMessage: {
|
||||
role: 'assistant',
|
||||
content: [
|
||||
{ type: 'tool_use', id: 'tool-live', name: 'grep_live', input: { pattern: 'TODO' } },
|
||||
],
|
||||
},
|
||||
streamingTools: [
|
||||
{
|
||||
toolCallId: 'tool-live',
|
||||
name: 'grep_live',
|
||||
status: 'running',
|
||||
updatedAt: Date.now(),
|
||||
summary: 'Scanning current workspace',
|
||||
},
|
||||
],
|
||||
sending: true,
|
||||
pendingFinal: false,
|
||||
showThinking: false,
|
||||
});
|
||||
|
||||
expect(steps).toHaveLength(8);
|
||||
expect(steps.at(-1)).toEqual(expect.objectContaining({
|
||||
id: 'tool-live',
|
||||
label: 'grep_live',
|
||||
status: 'running',
|
||||
}));
|
||||
});
|
||||
|
||||
it('keeps recent completed steps from assistant history', () => {
|
||||
const messages: RawMessage[] = [
|
||||
{
|
||||
@@ -70,14 +193,12 @@ describe('deriveTaskSteps', () => {
|
||||
label: 'Thinking',
|
||||
status: 'completed',
|
||||
kind: 'thinking',
|
||||
depth: 1,
|
||||
}),
|
||||
expect.objectContaining({
|
||||
id: 'tool-2',
|
||||
label: 'read_file',
|
||||
status: 'completed',
|
||||
kind: 'tool',
|
||||
depth: 1,
|
||||
}),
|
||||
]);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user