Ling0925
/
buster
mirror of https://github.com/buster-so/buster.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #812 from buster-so/hot-fix-post-processing-logic-wrong 

Add Slack notification handling logic in message post-processing task
This commit is contained in:
dal 2025-09-09 09:25:48 -06:00 committed by GitHub
parent 1dcd2c92d5 4c084922da
commit 7f43a92de3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 726 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

303
apps/trigger/src/tasks/message-post-processing/message-post-processing.test.ts
View File

@ -367,6 +367,309 @@ describe('messagePostProcessingTask', () => {
});
});
describe('Slack notification skip logic', () => {
it('should skip Slack notification when no issues found AND no major assumptions', async () => {
const messageId = '123e4567-e89b-12d3-a456-426614174000';
const workflowOutput = {
flagChatResult: {
type: 'noIssuesFound' as const,
message: 'No issues found',
},
assumptionsResult: {
toolCalled: 'noAssumptions',
assumptions: undefined,
},
formattedMessage: undefined,
};
vi.mocked(helpers.fetchMessageWithContext).mockResolvedValue({
id: messageId,
chatId: 'chat-123',
createdBy: 'user-123',
createdAt: new Date(),
rawLlmMessages: [] as any,
userName: 'John Doe',
organizationId: 'org-123',
});
vi.mocked(helpers.fetchPreviousPostProcessingMessages).mockResolvedValue([]);
vi.mocked(helpers.fetchUserDatasets).mockResolvedValue({
datasets: [],
total: 0,
page: 0,
pageSize: 1000,
});
vi.mocked(helpers.getExistingSlackMessageForChat).mockResolvedValue({ exists: false });
vi.mocked(helpers.buildWorkflowInput).mockReturnValue({
conversationHistory: undefined,
userName: 'John Doe',
isFollowUp: false,
isSlackFollowUp: false,
datasets: '',
});
vi.mocked(postProcessingWorkflow).mockResolvedValue(workflowOutput);
await runTask({ messageId });
// Verify Slack notification was NOT sent
expect(helpers.sendSlackNotification).not.toHaveBeenCalled();
expect(helpers.sendSlackReplyNotification).not.toHaveBeenCalled();
});
it('should skip Slack notification with only minor assumptions', async () => {
const messageId = '123e4567-e89b-12d3-a456-426614174000';
const workflowOutput = {
flagChatResult: {
type: 'noIssuesFound' as const,
message: 'No issues found',
},
assumptionsResult: {
toolCalled: 'listAssumptions',
assumptions: [
{
descriptiveTitle: 'Minor assumption',
classification: 'fieldMapping',
explanation: 'Test explanation',
label: 'minor' as const,
},
],
},
formattedMessage: undefined,
};
vi.mocked(helpers.fetchMessageWithContext).mockResolvedValue({
id: messageId,
chatId: 'chat-123',
createdBy: 'user-123',
createdAt: new Date(),
rawLlmMessages: [] as any,
userName: 'John Doe',
organizationId: 'org-123',
});
vi.mocked(helpers.fetchPreviousPostProcessingMessages).mockResolvedValue([]);
vi.mocked(helpers.fetchUserDatasets).mockResolvedValue({
datasets: [],
total: 0,
page: 0,
pageSize: 1000,
});
vi.mocked(helpers.getExistingSlackMessageForChat).mockResolvedValue({ exists: false });
vi.mocked(helpers.buildWorkflowInput).mockReturnValue({
conversationHistory: undefined,
userName: 'John Doe',
isFollowUp: false,
isSlackFollowUp: false,
datasets: '',
});
vi.mocked(postProcessingWorkflow).mockResolvedValue(workflowOutput);
await runTask({ messageId });
// Verify Slack notification was NOT sent (only minor assumptions)
expect(helpers.sendSlackNotification).not.toHaveBeenCalled();
expect(helpers.sendSlackReplyNotification).not.toHaveBeenCalled();
});
it('should send Slack notification when flagChat even without major assumptions', async () => {
const messageId = '123e4567-e89b-12d3-a456-426614174000';
const workflowOutput = {
flagChatResult: {
type: 'flagChat' as const,
summaryMessage: 'Issues found',
summaryTitle: 'Issue Title',
},
assumptionsResult: {
toolCalled: 'noAssumptions',
assumptions: undefined,
},
formattedMessage: 'Formatted message',
};
vi.mocked(helpers.fetchMessageWithContext).mockResolvedValue({
id: messageId,
chatId: 'chat-123',
createdBy: 'user-123',
createdAt: new Date(),
rawLlmMessages: [] as any,
userName: 'John Doe',
organizationId: 'org-123',
});
vi.mocked(helpers.fetchPreviousPostProcessingMessages).mockResolvedValue([]);
vi.mocked(helpers.fetchUserDatasets).mockResolvedValue({
datasets: [],
total: 0,
page: 0,
pageSize: 1000,
});
vi.mocked(helpers.getExistingSlackMessageForChat).mockResolvedValue({ exists: false });
vi.mocked(helpers.sendSlackNotification).mockResolvedValue({
sent: true,
messageTs: 'msg-ts-123',
integrationId: 'int-123',
channelId: 'C123456',
});
vi.mocked(helpers.buildWorkflowInput).mockReturnValue({
conversationHistory: undefined,
userName: 'John Doe',
isFollowUp: false,
isSlackFollowUp: false,
datasets: '',
});
vi.mocked(postProcessingWorkflow).mockResolvedValue(workflowOutput);
await runTask({ messageId });
// Verify Slack notification WAS sent (flagChat type)
expect(helpers.sendSlackNotification).toHaveBeenCalledWith({
organizationId: 'org-123',
userName: 'John Doe',
chatId: 'chat-123',
summaryTitle: 'Issue Title',
summaryMessage: 'Issues found',
toolCalled: 'noAssumptions',
});
});
it('should send Slack notification when major assumptions exist even with noIssuesFound', async () => {
const messageId = '123e4567-e89b-12d3-a456-426614174000';
const workflowOutput = {
flagChatResult: {
type: 'noIssuesFound' as const,
message: 'No issues',
},
assumptionsResult: {
toolCalled: 'listAssumptions',
assumptions: [
{
descriptiveTitle: 'Major assumption',
classification: 'fieldMapping',
explanation: 'Test',
label: 'major' as const,
},
],
},
formattedMessage: 'Major assumptions found',
};
vi.mocked(helpers.fetchMessageWithContext).mockResolvedValue({
id: messageId,
chatId: 'chat-123',
createdBy: 'user-123',
createdAt: new Date(),
rawLlmMessages: [] as any,
userName: 'John Doe',
organizationId: 'org-123',
});
vi.mocked(helpers.fetchPreviousPostProcessingMessages).mockResolvedValue([]);
vi.mocked(helpers.fetchUserDatasets).mockResolvedValue({
datasets: [],
total: 0,
page: 0,
pageSize: 1000,
});
vi.mocked(helpers.getExistingSlackMessageForChat).mockResolvedValue({ exists: false });
vi.mocked(helpers.sendSlackNotification).mockResolvedValue({
sent: true,
messageTs: 'msg-ts-123',
integrationId: 'int-123',
channelId: 'C123456',
});
vi.mocked(helpers.buildWorkflowInput).mockReturnValue({
conversationHistory: undefined,
userName: 'John Doe',
isFollowUp: false,
isSlackFollowUp: false,
datasets: '',
});
vi.mocked(postProcessingWorkflow).mockResolvedValue(workflowOutput);
await runTask({ messageId });
// Verify Slack notification WAS sent (major assumptions exist)
expect(helpers.sendSlackNotification).toHaveBeenCalled();
});
it('should handle undefined isSlackFollowUp in workflow input correctly', async () => {
const messageId = '123e4567-e89b-12d3-a456-426614174000';
const workflowOutput = {
flagChatResult: {
type: 'flagChat' as const,
summaryMessage: 'Issues found',
summaryTitle: 'Issue Title',
},
assumptionsResult: {
toolCalled: 'listAssumptions',
assumptions: [
{
descriptiveTitle: 'Major assumption',
classification: 'fieldMapping',
explanation: 'Test',
label: 'major' as const,
},
],
},
formattedMessage: 'Formatted message from workflow',
};
vi.mocked(helpers.fetchMessageWithContext).mockResolvedValue({
id: messageId,
chatId: 'chat-123',
createdBy: 'user-123',
createdAt: new Date(),
rawLlmMessages: [] as any,
userName: 'John Doe',
organizationId: 'org-123',
});
vi.mocked(helpers.fetchPreviousPostProcessingMessages).mockResolvedValue([]);
vi.mocked(helpers.fetchUserDatasets).mockResolvedValue({
datasets: [],
total: 0,
page: 0,
pageSize: 1000,
});
vi.mocked(helpers.getExistingSlackMessageForChat).mockResolvedValue({ exists: false });
vi.mocked(helpers.sendSlackNotification).mockResolvedValue({
sent: true,
messageTs: 'msg-ts-123',
integrationId: 'int-123',
channelId: 'C123456',
});
vi.mocked(helpers.buildWorkflowInput).mockReturnValue({
conversationHistory: undefined,
userName: 'John Doe',
isFollowUp: false,
isSlackFollowUp: undefined, // Critical: undefined case
datasets: '',
});
vi.mocked(postProcessingWorkflow).mockResolvedValue(workflowOutput);
await runTask({ messageId });
// Verify workflow was called with undefined isSlackFollowUp
expect(postProcessingWorkflow).toHaveBeenCalledWith({
conversationHistory: undefined,
userName: 'John Doe',
isFollowUp: false,
isSlackFollowUp: undefined,
datasets: '',
});
// Verify Slack notification WAS sent with formatted message
expect(helpers.sendSlackNotification).toHaveBeenCalledWith({
organizationId: 'org-123',
userName: 'John Doe',
chatId: 'chat-123',
summaryTitle: 'Issue Title',
summaryMessage: 'Issues found',
toolCalled: 'listAssumptions',
});
});
});
it('should return error result for database update failure', async () => {
const messageId = '123e4567-e89b-12d3-a456-426614174000';
const dbError = new Error('Database update failed');

4
apps/trigger/src/tasks/message-post-processing/message-post-processing.ts
View File

@ -234,11 +234,11 @@ export const messagePostProcessingTask: ReturnType<
// Step 7: Send Slack notification if conditions are met
let slackNotificationSent = false;
// Skip Slack notification if tool_called is "noIssuesFound" and there are no major assumptions
// Skip Slack notification if no issues were flagged and there are no major assumptions
const hasMajorAssumptions =
dbData.assumptions?.some((assumption) => assumption.label === 'major') ?? false;
const shouldSkipSlackNotification =
dbData.tool_called === 'noIssuesFound' && !hasMajorAssumptions;
validatedOutput.flagChatResult.type === 'noIssuesFound' && !hasMajorAssumptions;
try {
logger.log('Checking Slack notification conditions', {

419
packages/ai/src/workflows/message-post-processing-workflow/message-post-processing-workflow.test.ts Normal file
View File

@ -0,0 +1,419 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { runMessagePostProcessingWorkflow } from './message-post-processing-workflow';
import type { PostProcessingWorkflowInput } from './message-post-processing-workflow';
// Mock the steps
vi.mock('../../steps/message-post-processing-steps/flag-chat-step/flag-chat-step', () => ({
runFlagChatStep: vi.fn(),
}));
vi.mock(
'../../steps/message-post-processing-steps/identify-assumptions-step/identify-assumptions-step',
() => ({
runIdentifyAssumptionsStep: vi.fn(),
})
);
vi.mock(
'../../steps/message-post-processing-steps/format-follow-up-message-step/format-follow-up-message-step',
() => ({
runFormatFollowUpMessageStep: vi.fn(),
})
);
vi.mock(
'../../steps/message-post-processing-steps/format-initial-message-step/format-initial-message-step',
() => ({
runFormatInitialMessageStep: vi.fn(),
})
);
import { runFlagChatStep } from '../../steps/message-post-processing-steps/flag-chat-step/flag-chat-step';
import { runFormatFollowUpMessageStep } from '../../steps/message-post-processing-steps/format-follow-up-message-step/format-follow-up-message-step';
import { runFormatInitialMessageStep } from '../../steps/message-post-processing-steps/format-initial-message-step/format-initial-message-step';
import { runIdentifyAssumptionsStep } from '../../steps/message-post-processing-steps/identify-assumptions-step/identify-assumptions-step';
describe('runMessagePostProcessingWorkflow', () => {
beforeEach(() => {
vi.clearAllMocks();
});
describe('Formatting logic edge cases', () => {
it('should format initial message when isSlackFollowUp is undefined', async () => {
// Setup
const input: PostProcessingWorkflowInput = {
userName: 'Test User',
datasets: 'test datasets',
isFollowUp: false,
isSlackFollowUp: undefined, // Critical test case
};
vi.mocked(runFlagChatStep).mockResolvedValue({
type: 'flagChat',
summaryMessage: 'Issues found',
summaryTitle: 'Test Issue',
});
vi.mocked(runIdentifyAssumptionsStep).mockResolvedValue({
toolCalled: 'listAssumptions',
assumptions: [
{
descriptiveTitle: 'Major assumption',
classification: 'fieldMapping',
explanation: 'Test explanation',
label: 'major',
},
],
});
vi.mocked(runFormatInitialMessageStep).mockResolvedValue({
summaryMessage: 'Formatted initial message',
summaryTitle: 'Initial Title',
});
// Execute
const result = await runMessagePostProcessingWorkflow(input);
// Verify - should call format initial message when isSlackFollowUp is undefined
expect(runFormatInitialMessageStep).toHaveBeenCalledWith({
userName: 'Test User',
flaggedIssues: 'Issues found',
majorAssumptions: [
{
descriptiveTitle: 'Major assumption',
classification: 'fieldMapping',
explanation: 'Test explanation',
label: 'major',
},
],
conversationHistory: undefined,
});
expect(result.formattedMessage).toBe('Formatted initial message');
});
it('should format initial message when isSlackFollowUp is false', async () => {
// Setup
const input: PostProcessingWorkflowInput = {
userName: 'Test User',
datasets: 'test datasets',
isFollowUp: false,
isSlackFollowUp: false, // Explicit false
};
vi.mocked(runFlagChatStep).mockResolvedValue({
type: 'flagChat',
summaryMessage: 'Issues found',
summaryTitle: 'Test Issue',
});
vi.mocked(runIdentifyAssumptionsStep).mockResolvedValue({
toolCalled: 'listAssumptions',
assumptions: [
{
descriptiveTitle: 'Major assumption',
classification: 'fieldMapping',
explanation: 'Test explanation',
label: 'major',
},
],
});
vi.mocked(runFormatInitialMessageStep).mockResolvedValue({
summaryMessage: 'Formatted initial message',
summaryTitle: 'Initial Title',
});
// Execute
const result = await runMessagePostProcessingWorkflow(input);
// Verify
expect(runFormatInitialMessageStep).toHaveBeenCalled();
expect(result.formattedMessage).toBe('Formatted initial message');
});
it('should format follow-up message when both isFollowUp and isSlackFollowUp are true', async () => {
// Setup
const input: PostProcessingWorkflowInput = {
userName: 'Test User',
datasets: 'test datasets',
isFollowUp: true,
isSlackFollowUp: true,
};
vi.mocked(runFlagChatStep).mockResolvedValue({
type: 'flagChat',
summaryMessage: 'Issues found',
summaryTitle: 'Test Issue',
});
vi.mocked(runIdentifyAssumptionsStep).mockResolvedValue({
toolCalled: 'listAssumptions',
assumptions: [
{
descriptiveTitle: 'Major assumption',
classification: 'fieldMapping',
explanation: 'Test explanation',
label: 'major',
},
],
});
vi.mocked(runFormatFollowUpMessageStep).mockResolvedValue({
summaryMessage: 'Formatted follow-up message',
summaryTitle: 'Follow-up Title',
});
// Execute
const result = await runMessagePostProcessingWorkflow(input);
// Verify
expect(runFormatFollowUpMessageStep).toHaveBeenCalled();
expect(runFormatInitialMessageStep).not.toHaveBeenCalled();
expect(result.formattedMessage).toBe('Formatted follow-up message');
});
it('should NOT format message when no major assumptions and no flagged issues', async () => {
// Setup
const input: PostProcessingWorkflowInput = {
userName: 'Test User',
datasets: 'test datasets',
isFollowUp: false,
isSlackFollowUp: false,
};
vi.mocked(runFlagChatStep).mockResolvedValue({
type: 'noIssuesFound',
message: 'No issues found',
});
vi.mocked(runIdentifyAssumptionsStep).mockResolvedValue({
toolCalled: 'noAssumptions',
assumptions: undefined,
});
// Execute
const result = await runMessagePostProcessingWorkflow(input);
// Verify - no formatting functions should be called
expect(runFormatInitialMessageStep).not.toHaveBeenCalled();
expect(runFormatFollowUpMessageStep).not.toHaveBeenCalled();
expect(result.formattedMessage).toBeUndefined();
});
it('should format message when major assumptions exist even without flagged issues', async () => {
// Setup
const input: PostProcessingWorkflowInput = {
userName: 'Test User',
datasets: 'test datasets',
isFollowUp: false,
isSlackFollowUp: false,
};
vi.mocked(runFlagChatStep).mockResolvedValue({
type: 'noIssuesFound',
message: 'No issues found',
});
vi.mocked(runIdentifyAssumptionsStep).mockResolvedValue({
toolCalled: 'listAssumptions',
assumptions: [
{
descriptiveTitle: 'Major assumption',
classification: 'fieldMapping',
explanation: 'Test explanation',
label: 'major',
},
],
});
vi.mocked(runFormatInitialMessageStep).mockResolvedValue({
summaryMessage: 'Major assumptions require attention',
summaryTitle: 'Assumptions Found',
});
// Execute
const result = await runMessagePostProcessingWorkflow(input);
// Verify - should format because of major assumptions
expect(runFormatInitialMessageStep).toHaveBeenCalled();
expect(result.formattedMessage).toBe('Major assumptions require attention');
});
it('should NOT format message with only minor assumptions', async () => {
// Setup
const input: PostProcessingWorkflowInput = {
userName: 'Test User',
datasets: 'test datasets',
isFollowUp: false,
isSlackFollowUp: false,
};
vi.mocked(runFlagChatStep).mockResolvedValue({
type: 'noIssuesFound',
message: 'No issues found',
});
vi.mocked(runIdentifyAssumptionsStep).mockResolvedValue({
toolCalled: 'listAssumptions',
assumptions: [
{
descriptiveTitle: 'Minor assumption',
classification: 'fieldMapping',
explanation: 'Test explanation',
label: 'minor',
},
],
});
// Execute
const result = await runMessagePostProcessingWorkflow(input);
// Verify - should NOT format because only minor assumptions
expect(runFormatInitialMessageStep).not.toHaveBeenCalled();
expect(runFormatFollowUpMessageStep).not.toHaveBeenCalled();
expect(result.formattedMessage).toBeUndefined();
});
});
describe('Result structure validation', () => {
it('should return correct structure for flagChat with assumptions', async () => {
const input: PostProcessingWorkflowInput = {
userName: 'Test User',
datasets: 'test datasets',
};
vi.mocked(runFlagChatStep).mockResolvedValue({
type: 'flagChat',
summaryMessage: 'Issues found',
summaryTitle: 'Test Issue',
});
vi.mocked(runIdentifyAssumptionsStep).mockResolvedValue({
toolCalled: 'listAssumptions',
assumptions: [
{
descriptiveTitle: 'Test assumption',
classification: 'fieldMapping',
explanation: 'Test explanation',
label: 'major',
},
],
});
vi.mocked(runFormatInitialMessageStep).mockResolvedValue({
summaryMessage: 'Formatted message',
summaryTitle: 'Title',
});
const result = await runMessagePostProcessingWorkflow(input);
expect(result).toEqual({
flagChatResult: {
type: 'flagChat',
summaryMessage: 'Issues found',
summaryTitle: 'Test Issue',
message: undefined,
},
assumptionsResult: {
toolCalled: 'listAssumptions',
assumptions: [
{
descriptiveTitle: 'Test assumption',
classification: 'fieldMapping',
explanation: 'Test explanation',
label: 'major',
},
],
},
formattedMessage: 'Formatted message',
});
});
it('should return correct structure for noIssuesFound with no assumptions', async () => {
const input: PostProcessingWorkflowInput = {
userName: 'Test User',
datasets: 'test datasets',
};
vi.mocked(runFlagChatStep).mockResolvedValue({
type: 'noIssuesFound',
message: 'All good',
});
vi.mocked(runIdentifyAssumptionsStep).mockResolvedValue({
toolCalled: 'noAssumptions',
assumptions: undefined,
});
const result = await runMessagePostProcessingWorkflow(input);
expect(result).toEqual({
flagChatResult: {
type: 'noIssuesFound',
summaryMessage: undefined,
summaryTitle: undefined,
message: 'All good',
},
assumptionsResult: {
toolCalled: 'noAssumptions',
assumptions: undefined,
},
formattedMessage: undefined,
});
});
});
describe('Edge cases', () => {
it('should handle all undefined optional fields', async () => {
const input: PostProcessingWorkflowInput = {
userName: 'Test User',
datasets: '',
conversationHistory: undefined,
isFollowUp: undefined,
isSlackFollowUp: undefined,
};
vi.mocked(runFlagChatStep).mockResolvedValue({
type: 'noIssuesFound',
message: 'No issues',
});
vi.mocked(runIdentifyAssumptionsStep).mockResolvedValue({
toolCalled: 'noAssumptions',
assumptions: undefined,
});
// Should not throw
const result = await runMessagePostProcessingWorkflow(input);
expect(result).toBeDefined();
expect(result.formattedMessage).toBeUndefined();
});
it('should handle empty datasets string', async () => {
const input: PostProcessingWorkflowInput = {
userName: 'Test User',
datasets: '',
};
vi.mocked(runFlagChatStep).mockResolvedValue({
type: 'noIssuesFound',
message: 'No issues',
});
vi.mocked(runIdentifyAssumptionsStep).mockResolvedValue({
toolCalled: 'noAssumptions',
assumptions: undefined,
});
const result = await runMessagePostProcessingWorkflow(input);
expect(runFlagChatStep).toHaveBeenCalledWith({
conversationHistory: undefined,
userName: 'Test User',
datasets: '',
});
});
});
});

4
packages/ai/src/workflows/message-post-processing-workflow/message-post-processing-workflow.ts
View File

@ -91,8 +91,8 @@ export async function runMessagePostProcessingWorkflow(
conversationHistory: validatedInput.conversationHistory,
});
formattedMessage = followUpResult.summaryMessage;
} else if (validatedInput.isSlackFollowUp === false) {
// Format initial message
} else if (!validatedInput.isSlackFollowUp) {
// Format initial message (when isSlackFollowUp is false or undefined)
const initialResult = await runFormatInitialMessageStep({
userName: validatedInput.userName,
flaggedIssues,