fix(chat): hide internal system messages from webchat (#710)
This commit is contained in:
committed by
GitHub
Unverified
parent
47c9560b9e
commit
ef3cf64484
1
.gitignore
vendored
1
.gitignore
vendored
@@ -67,3 +67,4 @@ docs/pr-session-notes-*.md
|
|||||||
|
|
||||||
.cursor/
|
.cursor/
|
||||||
.pnpm-store/
|
.pnpm-store/
|
||||||
|
package-lock.json
|
||||||
|
|||||||
@@ -777,6 +777,16 @@ function isToolResultRole(role: unknown): boolean {
|
|||||||
return normalized === 'toolresult' || normalized === 'tool_result';
|
return normalized === 'toolresult' || normalized === 'tool_result';
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** True for internal plumbing messages that should never be shown in the UI. */
|
||||||
|
function isInternalMessage(msg: { role?: unknown; content?: unknown }): boolean {
|
||||||
|
if (msg.role === 'system') return true;
|
||||||
|
if (msg.role === 'assistant') {
|
||||||
|
const text = getMessageText(msg.content);
|
||||||
|
if (/^(HEARTBEAT_OK|NO_REPLY)\s*$/.test(text)) return true;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
function extractTextFromContent(content: unknown): string {
|
function extractTextFromContent(content: unknown): string {
|
||||||
if (typeof content === 'string') return content;
|
if (typeof content === 'string') return content;
|
||||||
if (!Array.isArray(content)) return '';
|
if (!Array.isArray(content)) return '';
|
||||||
@@ -1314,7 +1324,7 @@ export const useChatStore = create<ChatState>((set, get) => ({
|
|||||||
|
|
||||||
// Before filtering: attach images/files from tool_result messages to the next assistant message
|
// Before filtering: attach images/files from tool_result messages to the next assistant message
|
||||||
const messagesWithToolImages = enrichWithToolResultFiles(rawMessages);
|
const messagesWithToolImages = enrichWithToolResultFiles(rawMessages);
|
||||||
const filteredMessages = messagesWithToolImages.filter((msg) => !isToolResultRole(msg.role));
|
const filteredMessages = messagesWithToolImages.filter((msg) => !isToolResultRole(msg.role) && !isInternalMessage(msg));
|
||||||
// Restore file attachments for user/assistant messages (from cache + text patterns)
|
// Restore file attachments for user/assistant messages (from cache + text patterns)
|
||||||
const enrichedMessages = enrichWithCachedImages(filteredMessages);
|
const enrichedMessages = enrichWithCachedImages(filteredMessages);
|
||||||
|
|
||||||
|
|||||||
@@ -598,6 +598,16 @@ function isToolResultRole(role: unknown): boolean {
|
|||||||
return normalized === 'toolresult' || normalized === 'tool_result';
|
return normalized === 'toolresult' || normalized === 'tool_result';
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** True for internal plumbing messages that should never be shown in the UI. */
|
||||||
|
function isInternalMessage(msg: { role?: unknown; content?: unknown }): boolean {
|
||||||
|
if (msg.role === 'system') return true;
|
||||||
|
if (msg.role === 'assistant') {
|
||||||
|
const text = getMessageText(msg.content);
|
||||||
|
if (/^(HEARTBEAT_OK|NO_REPLY)\s*$/.test(text)) return true;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
function extractTextFromContent(content: unknown): string {
|
function extractTextFromContent(content: unknown): string {
|
||||||
if (typeof content === 'string') return content;
|
if (typeof content === 'string') return content;
|
||||||
if (!Array.isArray(content)) return '';
|
if (!Array.isArray(content)) return '';
|
||||||
@@ -824,6 +834,7 @@ export {
|
|||||||
extractRawFilePaths,
|
extractRawFilePaths,
|
||||||
makeAttachedFile,
|
makeAttachedFile,
|
||||||
enrichWithToolResultFiles,
|
enrichWithToolResultFiles,
|
||||||
|
isInternalMessage,
|
||||||
isToolResultRole,
|
isToolResultRole,
|
||||||
enrichWithCachedImages,
|
enrichWithCachedImages,
|
||||||
loadMissingPreviews,
|
loadMissingPreviews,
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ import {
|
|||||||
enrichWithToolResultFiles,
|
enrichWithToolResultFiles,
|
||||||
getMessageText,
|
getMessageText,
|
||||||
hasNonToolAssistantContent,
|
hasNonToolAssistantContent,
|
||||||
|
isInternalMessage,
|
||||||
isToolResultRole,
|
isToolResultRole,
|
||||||
loadMissingPreviews,
|
loadMissingPreviews,
|
||||||
toMs,
|
toMs,
|
||||||
@@ -39,7 +40,7 @@ export function createHistoryActions(
|
|||||||
const applyLoadedMessages = (rawMessages: RawMessage[], thinkingLevel: string | null) => {
|
const applyLoadedMessages = (rawMessages: RawMessage[], thinkingLevel: string | null) => {
|
||||||
// Before filtering: attach images/files from tool_result messages to the next assistant message
|
// Before filtering: attach images/files from tool_result messages to the next assistant message
|
||||||
const messagesWithToolImages = enrichWithToolResultFiles(rawMessages);
|
const messagesWithToolImages = enrichWithToolResultFiles(rawMessages);
|
||||||
const filteredMessages = messagesWithToolImages.filter((msg) => !isToolResultRole(msg.role));
|
const filteredMessages = messagesWithToolImages.filter((msg) => !isToolResultRole(msg.role) && !isInternalMessage(msg));
|
||||||
// Restore file attachments for user/assistant messages (from cache + text patterns)
|
// Restore file attachments for user/assistant messages (from cache + text patterns)
|
||||||
const enrichedMessages = enrichWithCachedImages(filteredMessages);
|
const enrichedMessages = enrichWithCachedImages(filteredMessages);
|
||||||
|
|
||||||
|
|||||||
@@ -11,6 +11,14 @@ const hasNonToolAssistantContent = vi.fn((message: { content?: unknown } | undef
|
|||||||
return typeof message.content === 'string' ? message.content.trim().length > 0 : true;
|
return typeof message.content === 'string' ? message.content.trim().length > 0 : true;
|
||||||
});
|
});
|
||||||
const isToolResultRole = vi.fn((role: unknown) => role === 'toolresult' || role === 'tool_result');
|
const isToolResultRole = vi.fn((role: unknown) => role === 'toolresult' || role === 'tool_result');
|
||||||
|
const isInternalMessage = vi.fn((msg: { role?: unknown; content?: unknown }) => {
|
||||||
|
if (msg.role === 'system') return true;
|
||||||
|
if (msg.role === 'assistant') {
|
||||||
|
const text = typeof msg.content === 'string' ? msg.content : '';
|
||||||
|
if (/^(HEARTBEAT_OK|NO_REPLY)\s*$/.test(text)) return true;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
});
|
||||||
const loadMissingPreviews = vi.fn(async () => false);
|
const loadMissingPreviews = vi.fn(async () => false);
|
||||||
const toMs = vi.fn((ts: number) => ts < 1e12 ? ts * 1000 : ts);
|
const toMs = vi.fn((ts: number) => ts < 1e12 ? ts * 1000 : ts);
|
||||||
|
|
||||||
@@ -28,6 +36,7 @@ vi.mock('@/stores/chat/helpers', () => ({
|
|||||||
enrichWithToolResultFiles: (...args: unknown[]) => enrichWithToolResultFiles(...args),
|
enrichWithToolResultFiles: (...args: unknown[]) => enrichWithToolResultFiles(...args),
|
||||||
getMessageText: (...args: unknown[]) => getMessageText(...args),
|
getMessageText: (...args: unknown[]) => getMessageText(...args),
|
||||||
hasNonToolAssistantContent: (...args: unknown[]) => hasNonToolAssistantContent(...args),
|
hasNonToolAssistantContent: (...args: unknown[]) => hasNonToolAssistantContent(...args),
|
||||||
|
isInternalMessage: (...args: unknown[]) => isInternalMessage(...args),
|
||||||
isToolResultRole: (...args: unknown[]) => isToolResultRole(...args),
|
isToolResultRole: (...args: unknown[]) => isToolResultRole(...args),
|
||||||
loadMissingPreviews: (...args: unknown[]) => loadMissingPreviews(...args),
|
loadMissingPreviews: (...args: unknown[]) => loadMissingPreviews(...args),
|
||||||
toMs: (...args: unknown[]) => toMs(...args as Parameters<typeof toMs>),
|
toMs: (...args: unknown[]) => toMs(...args as Parameters<typeof toMs>),
|
||||||
@@ -108,7 +117,6 @@ describe('chat history actions', () => {
|
|||||||
'/api/cron/session-history?sessionKey=agent%3Amain%3Acron%3Ajob-1&limit=200',
|
'/api/cron/session-history?sessionKey=agent%3Amain%3Acron%3Ajob-1&limit=200',
|
||||||
);
|
);
|
||||||
expect(h.read().messages.map((message) => message.content)).toEqual([
|
expect(h.read().messages.map((message) => message.content)).toEqual([
|
||||||
'Scheduled task: Drink water',
|
|
||||||
'Drink water 💧',
|
'Drink water 💧',
|
||||||
]);
|
]);
|
||||||
expect(h.read().sessionLastActivity['agent:main:cron:job-1']).toBe(1773281732751);
|
expect(h.read().sessionLastActivity['agent:main:cron:job-1']).toBe(1773281732751);
|
||||||
@@ -128,4 +136,99 @@ describe('chat history actions', () => {
|
|||||||
expect(h.read().messages).toEqual([]);
|
expect(h.read().messages).toEqual([]);
|
||||||
expect(h.read().loading).toBe(false);
|
expect(h.read().loading).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('filters out system messages from loaded history', async () => {
|
||||||
|
const { createHistoryActions } = await import('@/stores/chat/history-actions');
|
||||||
|
const h = makeHarness();
|
||||||
|
const actions = createHistoryActions(h.set as never, h.get as never);
|
||||||
|
|
||||||
|
invokeIpcMock.mockResolvedValueOnce({
|
||||||
|
success: true,
|
||||||
|
result: {
|
||||||
|
messages: [
|
||||||
|
{ role: 'user', content: 'Hello', timestamp: 1000 },
|
||||||
|
{ role: 'system', content: 'Gateway restarted', timestamp: 1001 },
|
||||||
|
{ role: 'assistant', content: 'Hi there!', timestamp: 1002 },
|
||||||
|
],
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
await actions.loadHistory();
|
||||||
|
|
||||||
|
expect(h.read().messages.map((m) => m.content)).toEqual([
|
||||||
|
'Hello',
|
||||||
|
'Hi there!',
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('filters out HEARTBEAT_OK assistant messages', async () => {
|
||||||
|
const { createHistoryActions } = await import('@/stores/chat/history-actions');
|
||||||
|
const h = makeHarness();
|
||||||
|
const actions = createHistoryActions(h.set as never, h.get as never);
|
||||||
|
|
||||||
|
invokeIpcMock.mockResolvedValueOnce({
|
||||||
|
success: true,
|
||||||
|
result: {
|
||||||
|
messages: [
|
||||||
|
{ role: 'user', content: 'Hello', timestamp: 1000 },
|
||||||
|
{ role: 'assistant', content: 'HEARTBEAT_OK', timestamp: 1001 },
|
||||||
|
{ role: 'assistant', content: 'Real response', timestamp: 1002 },
|
||||||
|
],
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
await actions.loadHistory();
|
||||||
|
|
||||||
|
expect(h.read().messages.map((m) => m.content)).toEqual([
|
||||||
|
'Hello',
|
||||||
|
'Real response',
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('filters out NO_REPLY assistant messages', async () => {
|
||||||
|
const { createHistoryActions } = await import('@/stores/chat/history-actions');
|
||||||
|
const h = makeHarness();
|
||||||
|
const actions = createHistoryActions(h.set as never, h.get as never);
|
||||||
|
|
||||||
|
invokeIpcMock.mockResolvedValueOnce({
|
||||||
|
success: true,
|
||||||
|
result: {
|
||||||
|
messages: [
|
||||||
|
{ role: 'user', content: 'Hello', timestamp: 1000 },
|
||||||
|
{ role: 'assistant', content: 'NO_REPLY', timestamp: 1001 },
|
||||||
|
{ role: 'assistant', content: 'Actual answer', timestamp: 1002 },
|
||||||
|
],
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
await actions.loadHistory();
|
||||||
|
|
||||||
|
expect(h.read().messages.map((m) => m.content)).toEqual([
|
||||||
|
'Hello',
|
||||||
|
'Actual answer',
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('keeps normal assistant messages that contain HEARTBEAT_OK as substring', async () => {
|
||||||
|
const { createHistoryActions } = await import('@/stores/chat/history-actions');
|
||||||
|
const h = makeHarness();
|
||||||
|
const actions = createHistoryActions(h.set as never, h.get as never);
|
||||||
|
|
||||||
|
invokeIpcMock.mockResolvedValueOnce({
|
||||||
|
success: true,
|
||||||
|
result: {
|
||||||
|
messages: [
|
||||||
|
{ role: 'user', content: 'What is HEARTBEAT_OK?', timestamp: 1000 },
|
||||||
|
{ role: 'assistant', content: 'HEARTBEAT_OK is a status code', timestamp: 1001 },
|
||||||
|
],
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
await actions.loadHistory();
|
||||||
|
|
||||||
|
expect(h.read().messages.map((m) => m.content)).toEqual([
|
||||||
|
'What is HEARTBEAT_OK?',
|
||||||
|
'HEARTBEAT_OK is a status code',
|
||||||
|
]);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user