From f0796b35afcd1bbf90ce03e3282fc4697ce2ce49 Mon Sep 17 00:00:00 2001 From: dal Date: Mon, 18 Aug 2025 23:42:27 -0600 Subject: [PATCH] Refactor message post-processing logic to improve handling of workflow outputs and assumptions. Updated test cases to reflect changes in message structure and added type definitions for flagChat results. Enhanced error handling for scenarios with minimal workflow output. --- .../helpers/data-transformers.test.ts | 6 --- .../helpers/data-transformers.ts | 9 +--- .../message-post-processing.test.ts | 54 ++++++++----------- .../message-post-processing.ts | 2 +- 4 files changed, 25 insertions(+), 46 deletions(-) diff --git a/apps/trigger/src/tasks/message-post-processing/helpers/data-transformers.test.ts b/apps/trigger/src/tasks/message-post-processing/helpers/data-transformers.test.ts index 8011c5353..793dd6aab 100644 --- a/apps/trigger/src/tasks/message-post-processing/helpers/data-transformers.test.ts +++ b/apps/trigger/src/tasks/message-post-processing/helpers/data-transformers.test.ts @@ -190,12 +190,8 @@ describe('data-transformers', () => { expect(result).toEqual({ conversationHistory: [{ role: 'user', content: 'Hello' }], userName: 'John Doe', - messageId: 'msg-123', - userId: 'user-123', - chatId: 'chat-123', isFollowUp: false, isSlackFollowUp: false, - previousMessages: [], datasets: 'yaml content', }); }); @@ -212,8 +208,6 @@ describe('data-transformers', () => { expect(result.isFollowUp).toBe(true); expect(result.isSlackFollowUp).toBe(true); - expect(result.previousMessages).toHaveLength(1); - expect(result.previousMessages[0]).toContain('Previous assumption'); }); it('should handle null userName', () => { diff --git a/apps/trigger/src/tasks/message-post-processing/helpers/data-transformers.ts b/apps/trigger/src/tasks/message-post-processing/helpers/data-transformers.ts index 85b13f0fd..cac84bc6d 100644 --- a/apps/trigger/src/tasks/message-post-processing/helpers/data-transformers.ts +++ b/apps/trigger/src/tasks/message-post-processing/helpers/data-transformers.ts @@ -1,6 +1,6 @@ import type { PermissionedDataset } from '@buster/access-controls'; import type { MessageHistory } from '@buster/ai/utils/memory/types'; -import type { PostProcessingWorkflowInput } from '@buster/ai/workflows/post-processing-workflow'; +import type { PostProcessingWorkflowInput } from '@buster/ai/workflows/message-post-processing-workflow/message-post-processing-workflow'; import type { MessageContext, PostProcessingResult } from '../types'; /** @@ -56,21 +56,14 @@ export function buildWorkflowInput( // Determine if this is a Slack follow-up (both follow-up AND Slack message exists) const isSlackFollowUp = isFollowUp && slackMessageExists; - // Format previous messages - const previousMessages = formatPreviousMessages(previousPostProcessingResults); - // Concatenate datasets const datasetsYaml = concatenateDatasets(datasets); return { conversationHistory, userName: messageContext.userName || 'Unknown User', - messageId: messageContext.id, - userId: messageContext.createdBy, - chatId: messageContext.chatId, isFollowUp, isSlackFollowUp, - previousMessages, datasets: datasetsYaml, }; } diff --git a/apps/trigger/src/tasks/message-post-processing/message-post-processing.test.ts b/apps/trigger/src/tasks/message-post-processing/message-post-processing.test.ts index 648e34b79..b26dac7f5 100644 --- a/apps/trigger/src/tasks/message-post-processing/message-post-processing.test.ts +++ b/apps/trigger/src/tasks/message-post-processing/message-post-processing.test.ts @@ -124,12 +124,13 @@ describe('messagePostProcessingTask', () => { descriptiveTitle: 'Test assumption', classification: 'business_rules', explanation: 'Test explanation', - label: 'important', + label: 'major' as const, }, ], toolCalled: 'analyze', }, flagChatResult: { + type: 'flagChat' as const, summaryTitle: 'Test Summary', summaryMessage: 'Test summary message', message: 'Test message', @@ -156,12 +157,8 @@ describe('messagePostProcessingTask', () => { vi.mocked(helpers.buildWorkflowInput).mockReturnValue({ conversationHistory: [{ role: 'user', content: 'Hello' }], userName: 'John Doe', - messageId, - userId: 'user-123', - chatId: 'chat-123', isFollowUp: false, isSlackFollowUp: false, - previousMessages: [], datasets: '', }); vi.mocked(postProcessingWorkflow).mockResolvedValue(workflowOutput); @@ -190,15 +187,15 @@ describe('messagePostProcessingTask', () => { expect(mockDb.update).toHaveBeenCalledWith(database.messages); expect(mockDb.set).toHaveBeenCalledWith({ postProcessingMessage: { - summary_message: 'Test message', - summary_title: 'No Major Assumptions Identified', - confidence_score: 'high', + summary_message: 'Test summary message', + summary_title: 'Test Summary', + confidence_score: 'low', assumptions: [ { descriptive_title: 'Test assumption', classification: 'business_rules', explanation: 'Test explanation', - label: 'important', + label: 'major' as const, }, ], tool_called: 'analyze', @@ -223,6 +220,7 @@ describe('messagePostProcessingTask', () => { toolCalled: 'analyze', }, flagChatResult: { + type: 'noIssuesFound' as const, summaryTitle: 'Follow-up Analysis', summaryMessage: 'Based on previous conversation...', message: 'Follow-up message', @@ -264,12 +262,8 @@ describe('messagePostProcessingTask', () => { vi.mocked(helpers.buildWorkflowInput).mockReturnValue({ conversationHistory: undefined, userName: 'John Doe', - messageId, - userId: 'user-123', - chatId: 'chat-123', isFollowUp: true, isSlackFollowUp: true, - previousMessages: ['{"assumptions":["Previous assumption"]}'], datasets: '', }); vi.mocked(postProcessingWorkflow).mockResolvedValue(workflowOutput); @@ -294,7 +288,7 @@ describe('messagePostProcessingTask', () => { ); }); - it('should return error result when workflow returns no output', async () => { + it('should handle workflow with minimal output', async () => { const messageId = '123e4567-e89b-12d3-a456-426614174000'; vi.mocked(helpers.fetchMessageWithContext).mockResolvedValue({ @@ -324,28 +318,29 @@ describe('messagePostProcessingTask', () => { vi.mocked(helpers.buildWorkflowInput).mockReturnValue({ conversationHistory: undefined, userName: 'John Doe', - messageId, - userId: 'user-123', - chatId: 'chat-123', isFollowUp: false, isSlackFollowUp: false, - previousMessages: [], datasets: '', }); - vi.mocked(postProcessingWorkflow).mockResolvedValue(null); + vi.mocked(postProcessingWorkflow).mockResolvedValue({ + flagChatResult: { + type: 'noIssuesFound' as const, + }, + assumptionsResult: { + toolCalled: 'none', + }, + }); const result = await runTask({ messageId }); expect(result).toEqual({ - success: false, + success: true, messageId, - error: { - code: 'WORKFLOW_EXECUTION_ERROR', - message: 'Post-processing workflow returned no output', - details: { - operation: 'message_post_processing_task_execution', - messageId, - }, + result: { + success: true, + messageId, + executionTimeMs: expect.any(Number), + workflowCompleted: true, }, }); }); @@ -403,12 +398,8 @@ describe('messagePostProcessingTask', () => { vi.mocked(helpers.buildWorkflowInput).mockReturnValue({ conversationHistory: undefined, userName: 'John Doe', - messageId, - userId: 'user-123', - chatId: 'chat-123', isFollowUp: false, isSlackFollowUp: false, - previousMessages: [], datasets: '', }); vi.mocked(postProcessingWorkflow).mockResolvedValue({ @@ -417,6 +408,7 @@ describe('messagePostProcessingTask', () => { toolCalled: 'analyze', }, flagChatResult: { + type: 'flagChat' as const, summaryTitle: 'Summary', summaryMessage: 'Summary message', message: 'Summary message', diff --git a/apps/trigger/src/tasks/message-post-processing/message-post-processing.ts b/apps/trigger/src/tasks/message-post-processing/message-post-processing.ts index a1a7fcdf9..6e214a41e 100644 --- a/apps/trigger/src/tasks/message-post-processing/message-post-processing.ts +++ b/apps/trigger/src/tasks/message-post-processing/message-post-processing.ts @@ -153,7 +153,7 @@ export const messagePostProcessingTask: ReturnType< messageId: payload.messageId, isFollowUp: workflowInput.isFollowUp, isSlackFollowUp: workflowInput.isSlackFollowUp, - previousMessagesCount: workflowInput.previousMessages.length, + previousMessagesCount: previousPostProcessingResults.length, hasConversationHistory: !!workflowInput.conversationHistory, datasetsLength: workflowInput.datasets.length, });