mirror of https://github.com/buster-so/buster.git
Merge pull request #461 from buster-so/dal/chat-history-in-summary-steps
add conversation history to the
This commit is contained in:
commit
4b5ffeee37
|
@ -131,12 +131,15 @@ export class SlackHandler {
|
|||
*/
|
||||
async handleOAuthCallback(c: Context): Promise<Response> {
|
||||
try {
|
||||
// Get base URL from environment
|
||||
const baseUrl = process.env.BUSTER_URL || '';
|
||||
|
||||
// Get service instance (lazy initialization)
|
||||
const slackOAuthService = this.getSlackOAuthService();
|
||||
|
||||
// Check if service is available
|
||||
if (!slackOAuthService) {
|
||||
return c.redirect('/app/settings/integrations?status=error&error=not_configured');
|
||||
return c.redirect(`${baseUrl}/app/settings/integrations?status=error&error=not_configured`);
|
||||
}
|
||||
|
||||
// Parse query parameters
|
||||
|
@ -153,7 +156,7 @@ export class SlackHandler {
|
|||
// Handle user denial
|
||||
if (query.error === 'access_denied') {
|
||||
console.info('OAuth flow cancelled by user');
|
||||
return c.redirect('/app/settings/integrations?status=cancelled');
|
||||
return c.redirect(`${baseUrl}/app/settings/integrations?status=cancelled`);
|
||||
}
|
||||
|
||||
console.error('Invalid OAuth callback parameters:', {
|
||||
|
@ -161,7 +164,9 @@ export class SlackHandler {
|
|||
providedKeys: Object.keys(query),
|
||||
expectedKeys: ['code', 'state'],
|
||||
});
|
||||
return c.redirect('/app/settings/integrations?status=error&error=invalid_parameters');
|
||||
return c.redirect(
|
||||
`${baseUrl}/app/settings/integrations?status=error&error=invalid_parameters`
|
||||
);
|
||||
}
|
||||
|
||||
// Handle OAuth callback
|
||||
|
@ -178,7 +183,7 @@ export class SlackHandler {
|
|||
resultKeys: Object.keys(result),
|
||||
});
|
||||
const errorParam = encodeURIComponent(result.error || 'unknown_error');
|
||||
return c.redirect(`/app/settings/integrations?status=error&error=${errorParam}`);
|
||||
return c.redirect(`${baseUrl}/app/settings/integrations?status=error&error=${errorParam}`);
|
||||
}
|
||||
|
||||
// Use metadata to determine return URL
|
||||
|
@ -187,7 +192,7 @@ export class SlackHandler {
|
|||
? `&workspace=${encodeURIComponent(result.teamName)}`
|
||||
: '';
|
||||
|
||||
return c.redirect(`${returnUrl}?status=success${workspaceParam}`);
|
||||
return c.redirect(`${baseUrl}${returnUrl}?status=success${workspaceParam}`);
|
||||
} catch (error) {
|
||||
console.error('OAuth callback error:', {
|
||||
errorType: error?.constructor?.name || 'Unknown',
|
||||
|
@ -197,7 +202,7 @@ export class SlackHandler {
|
|||
});
|
||||
const errorMessage = error instanceof Error ? error.message : 'callback_failed';
|
||||
return c.redirect(
|
||||
`/app/settings/integrations?status=error&error=${encodeURIComponent(errorMessage)}`
|
||||
`${baseUrl}/app/settings/integrations?status=error&error=${encodeURIComponent(errorMessage)}`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -126,6 +126,15 @@ ${
|
|||
: 'No major assumptions identified'
|
||||
}
|
||||
|
||||
${
|
||||
inputData.conversationHistory && inputData.conversationHistory.length > 0
|
||||
? `\nChat History:
|
||||
\`\`\`
|
||||
${JSON.stringify(inputData.conversationHistory, null, 2)}
|
||||
\`\`\``
|
||||
: ''
|
||||
}
|
||||
|
||||
Generate a concise update message for the data team.`;
|
||||
|
||||
const messages: CoreMessage[] = standardizeMessages(contextMessage);
|
||||
|
|
|
@ -160,6 +160,15 @@ ${
|
|||
: 'No major assumptions identified'
|
||||
}
|
||||
|
||||
${
|
||||
inputData.conversationHistory && inputData.conversationHistory.length > 0
|
||||
? `\nChat History:
|
||||
\`\`\`
|
||||
${JSON.stringify(inputData.conversationHistory, null, 2)}
|
||||
\`\`\``
|
||||
: ''
|
||||
}
|
||||
|
||||
Generate a cohesive summary with title for the data team.`;
|
||||
|
||||
const messages: CoreMessage[] = standardizeMessages(contextMessage);
|
||||
|
|
|
@ -0,0 +1,222 @@
|
|||
import { describe, expect, test, vi } from 'vitest';
|
||||
import type { CoreMessage } from 'ai';
|
||||
import { formatFollowUpMessageStepExecution } from '../../../src/steps/post-processing/format-follow-up-message-step';
|
||||
|
||||
// Mock the agent and its dependencies
|
||||
vi.mock('@mastra/core', () => ({
|
||||
Agent: vi.fn().mockImplementation(() => ({
|
||||
generate: vi.fn().mockResolvedValue({
|
||||
toolCalls: [{
|
||||
toolName: 'generateUpdateMessage',
|
||||
args: {
|
||||
update_message: 'Test update message',
|
||||
title: 'Update Title'
|
||||
}
|
||||
}]
|
||||
})
|
||||
})),
|
||||
createStep: vi.fn((config) => config)
|
||||
}));
|
||||
|
||||
vi.mock('braintrust', () => ({
|
||||
wrapTraced: vi.fn((fn) => fn)
|
||||
}));
|
||||
|
||||
vi.mock('../../../src/utils/models/anthropic-cached', () => ({
|
||||
anthropicCachedModel: vi.fn(() => 'mocked-model')
|
||||
}));
|
||||
|
||||
vi.mock('../../../src/utils/standardizeMessages', () => ({
|
||||
standardizeMessages: vi.fn((msg) => [{ role: 'user', content: msg }])
|
||||
}));
|
||||
|
||||
vi.mock('../../../src/tools/post-processing/generate-update-message', () => ({
|
||||
generateUpdateMessage: {}
|
||||
}));
|
||||
|
||||
describe('Format Follow-up Message Step Unit Tests', () => {
|
||||
test('should include chat history in context message when present', async () => {
|
||||
const mockConversationHistory: CoreMessage[] = [
|
||||
{ role: 'user', content: 'Initial query about sales' },
|
||||
{ role: 'assistant', content: 'Sales data analysis complete' },
|
||||
{ role: 'user', content: 'Can you filter by last 6 months?' },
|
||||
{ role: 'assistant', content: 'Filtered data shown' }
|
||||
];
|
||||
|
||||
const inputData = {
|
||||
conversationHistory: mockConversationHistory,
|
||||
userName: 'Sarah Connor',
|
||||
messageId: 'msg-fu-123',
|
||||
userId: 'user-123',
|
||||
chatId: 'chat-123',
|
||||
isFollowUp: true,
|
||||
isSlackFollowUp: true,
|
||||
previousMessages: ['Previous slack message'],
|
||||
datasets: 'test datasets',
|
||||
toolCalled: 'flagChat',
|
||||
flagChatMessage: 'New assumptions made during follow-up',
|
||||
flagChatTitle: 'Follow-up Issues',
|
||||
assumptions: [
|
||||
{
|
||||
descriptiveTitle: 'Time Period Filter',
|
||||
classification: 'timePeriodInterpretation' as const,
|
||||
explanation: 'Assumed last 6 months means from today',
|
||||
label: 'major' as const
|
||||
}
|
||||
]
|
||||
};
|
||||
|
||||
const result = await formatFollowUpMessageStepExecution({ inputData });
|
||||
|
||||
expect(result.summaryMessage).toBe('Test update message');
|
||||
expect(result.summaryTitle).toBe('Update Title');
|
||||
expect(result.message).toBe('Test update message');
|
||||
expect(result.conversationHistory).toEqual(mockConversationHistory);
|
||||
});
|
||||
|
||||
test('should handle empty conversation history', async () => {
|
||||
const inputData = {
|
||||
conversationHistory: [],
|
||||
userName: 'Kyle Reese',
|
||||
messageId: 'msg-fu-456',
|
||||
userId: 'user-456',
|
||||
chatId: 'chat-456',
|
||||
isFollowUp: true,
|
||||
isSlackFollowUp: false,
|
||||
previousMessages: [],
|
||||
datasets: 'test datasets',
|
||||
toolCalled: 'flagChat',
|
||||
flagChatMessage: 'Follow-up issues detected',
|
||||
flagChatTitle: 'Issues Found',
|
||||
assumptions: [
|
||||
{
|
||||
descriptiveTitle: 'Data Aggregation',
|
||||
classification: 'aggregation' as const,
|
||||
explanation: 'Used SUM for totals',
|
||||
label: 'major' as const
|
||||
}
|
||||
]
|
||||
};
|
||||
|
||||
const result = await formatFollowUpMessageStepExecution({ inputData });
|
||||
|
||||
expect(result.summaryMessage).toBe('Test update message');
|
||||
expect(result.summaryTitle).toBe('Update Title');
|
||||
expect(result.message).toBe('Test update message');
|
||||
expect(result.conversationHistory).toEqual([]);
|
||||
});
|
||||
|
||||
test('should handle missing conversation history', async () => {
|
||||
const inputData = {
|
||||
userName: 'John Connor',
|
||||
messageId: 'msg-fu-789',
|
||||
userId: 'user-789',
|
||||
chatId: 'chat-789',
|
||||
isFollowUp: true,
|
||||
isSlackFollowUp: true,
|
||||
previousMessages: ['Initial slack thread message'],
|
||||
datasets: 'test datasets',
|
||||
toolCalled: 'flagChat',
|
||||
flagChatMessage: 'Additional clarification needed',
|
||||
flagChatTitle: 'Needs Clarification',
|
||||
assumptions: [
|
||||
{
|
||||
descriptiveTitle: 'Metric Definition',
|
||||
classification: 'metricDefinition' as const,
|
||||
explanation: 'Used standard revenue metric',
|
||||
label: 'major' as const
|
||||
}
|
||||
]
|
||||
};
|
||||
|
||||
const result = await formatFollowUpMessageStepExecution({ inputData });
|
||||
|
||||
expect(result.summaryMessage).toBe('Test update message');
|
||||
expect(result.summaryTitle).toBe('Update Title');
|
||||
expect(result.message).toBe('Test update message');
|
||||
expect(result.conversationHistory).toBeUndefined();
|
||||
});
|
||||
|
||||
test('should return input data unchanged when no major assumptions', async () => {
|
||||
const inputData = {
|
||||
conversationHistory: [
|
||||
{ role: 'user' as const, content: 'Follow-up question' },
|
||||
{ role: 'assistant' as const, content: 'Follow-up answer' }
|
||||
],
|
||||
userName: 'Marcus Wright',
|
||||
messageId: 'msg-fu-999',
|
||||
userId: 'user-999',
|
||||
chatId: 'chat-999',
|
||||
isFollowUp: true,
|
||||
isSlackFollowUp: false,
|
||||
previousMessages: [],
|
||||
datasets: 'test datasets',
|
||||
toolCalled: 'noIssuesFound',
|
||||
assumptions: [
|
||||
{
|
||||
descriptiveTitle: 'Formatting Choice',
|
||||
classification: 'dataFormat' as const,
|
||||
explanation: 'Used comma separation',
|
||||
label: 'minor' as const
|
||||
},
|
||||
{
|
||||
descriptiveTitle: 'Time Zone',
|
||||
classification: 'timePeriodInterpretation' as const,
|
||||
explanation: 'Used UTC',
|
||||
label: 'timeRelated' as const
|
||||
}
|
||||
]
|
||||
};
|
||||
|
||||
const result = await formatFollowUpMessageStepExecution({ inputData });
|
||||
|
||||
expect(result).toEqual(inputData);
|
||||
expect(result.summaryMessage).toBeUndefined();
|
||||
expect(result.summaryTitle).toBeUndefined();
|
||||
expect(result.message).toBeUndefined();
|
||||
});
|
||||
|
||||
test('should handle multiple major assumptions with conversation history', async () => {
|
||||
const mockConversationHistory: CoreMessage[] = [
|
||||
{ role: 'user', content: 'Show me customer segments' },
|
||||
{ role: 'assistant', content: 'Here are the segments' },
|
||||
{ role: 'user', content: 'Filter by enterprise only' }
|
||||
];
|
||||
|
||||
const inputData = {
|
||||
conversationHistory: mockConversationHistory,
|
||||
userName: 'Kate Brewster',
|
||||
messageId: 'msg-fu-multi',
|
||||
userId: 'user-multi',
|
||||
chatId: 'chat-multi',
|
||||
isFollowUp: true,
|
||||
isSlackFollowUp: true,
|
||||
previousMessages: ['Thread about customer analysis'],
|
||||
datasets: 'test datasets',
|
||||
toolCalled: 'flagChat',
|
||||
flagChatMessage: 'Multiple assumptions in follow-up',
|
||||
flagChatTitle: 'Multiple Issues',
|
||||
assumptions: [
|
||||
{
|
||||
descriptiveTitle: 'Enterprise Definition',
|
||||
classification: 'segmentDefinition' as const,
|
||||
explanation: 'Defined enterprise as >$1M revenue',
|
||||
label: 'major' as const
|
||||
},
|
||||
{
|
||||
descriptiveTitle: 'Customer Status',
|
||||
classification: 'dataQuality' as const,
|
||||
explanation: 'Included only active customers',
|
||||
label: 'major' as const
|
||||
}
|
||||
]
|
||||
};
|
||||
|
||||
const result = await formatFollowUpMessageStepExecution({ inputData });
|
||||
|
||||
expect(result.summaryMessage).toBe('Test update message');
|
||||
expect(result.summaryTitle).toBe('Update Title');
|
||||
expect(result.message).toBe('Test update message');
|
||||
expect(result.conversationHistory).toEqual(mockConversationHistory);
|
||||
});
|
||||
});
|
|
@ -0,0 +1,166 @@
|
|||
import { describe, expect, test, vi } from 'vitest';
|
||||
import type { CoreMessage } from 'ai';
|
||||
import { formatInitialMessageStepExecution } from '../../../src/steps/post-processing/format-initial-message-step';
|
||||
|
||||
// Mock the agent and its dependencies
|
||||
vi.mock('@mastra/core', () => ({
|
||||
Agent: vi.fn().mockImplementation(() => ({
|
||||
generate: vi.fn().mockResolvedValue({
|
||||
toolCalls: [{
|
||||
toolName: 'generateSummary',
|
||||
args: {
|
||||
summary_message: 'Test summary message',
|
||||
title: 'Test Title'
|
||||
}
|
||||
}]
|
||||
})
|
||||
})),
|
||||
createStep: vi.fn((config) => config)
|
||||
}));
|
||||
|
||||
vi.mock('braintrust', () => ({
|
||||
wrapTraced: vi.fn((fn) => fn)
|
||||
}));
|
||||
|
||||
vi.mock('../../../src/utils/models/anthropic-cached', () => ({
|
||||
anthropicCachedModel: vi.fn(() => 'mocked-model')
|
||||
}));
|
||||
|
||||
vi.mock('../../../src/utils/standardizeMessages', () => ({
|
||||
standardizeMessages: vi.fn((msg) => [{ role: 'user', content: msg }])
|
||||
}));
|
||||
|
||||
vi.mock('../../../src/tools/post-processing/generate-summary', () => ({
|
||||
generateSummary: {}
|
||||
}));
|
||||
|
||||
describe('Format Initial Message Step Unit Tests', () => {
|
||||
test('should include chat history in context message when present', async () => {
|
||||
const mockConversationHistory: CoreMessage[] = [
|
||||
{ role: 'user', content: 'What is the total revenue?' },
|
||||
{ role: 'assistant', content: 'The total revenue is $1M' }
|
||||
];
|
||||
|
||||
const inputData = {
|
||||
conversationHistory: mockConversationHistory,
|
||||
userName: 'John Doe',
|
||||
messageId: 'msg-123',
|
||||
userId: 'user-123',
|
||||
chatId: 'chat-123',
|
||||
isFollowUp: false,
|
||||
isSlackFollowUp: false,
|
||||
previousMessages: [],
|
||||
datasets: 'test datasets',
|
||||
toolCalled: 'flagChat',
|
||||
flagChatMessage: 'Major issues found',
|
||||
flagChatTitle: 'Issues Detected',
|
||||
assumptions: [
|
||||
{
|
||||
descriptiveTitle: 'Revenue Calculation',
|
||||
classification: 'metricDefinition' as const,
|
||||
explanation: 'Assumed revenue includes all sources',
|
||||
label: 'major' as const
|
||||
}
|
||||
]
|
||||
};
|
||||
|
||||
const result = await formatInitialMessageStepExecution({ inputData });
|
||||
|
||||
expect(result.summaryMessage).toBe('Test summary message');
|
||||
expect(result.summaryTitle).toBe('Test Title');
|
||||
expect(result.conversationHistory).toEqual(mockConversationHistory);
|
||||
});
|
||||
|
||||
test('should handle empty conversation history', async () => {
|
||||
const inputData = {
|
||||
conversationHistory: [],
|
||||
userName: 'Jane Smith',
|
||||
messageId: 'msg-456',
|
||||
userId: 'user-456',
|
||||
chatId: 'chat-456',
|
||||
isFollowUp: false,
|
||||
isSlackFollowUp: false,
|
||||
previousMessages: [],
|
||||
datasets: 'test datasets',
|
||||
toolCalled: 'flagChat',
|
||||
flagChatMessage: 'Minor issues found',
|
||||
flagChatTitle: 'Minor Issues',
|
||||
assumptions: [
|
||||
{
|
||||
descriptiveTitle: 'Customer Count',
|
||||
classification: 'dataQuality' as const,
|
||||
explanation: 'Included all customer statuses',
|
||||
label: 'major' as const
|
||||
}
|
||||
]
|
||||
};
|
||||
|
||||
const result = await formatInitialMessageStepExecution({ inputData });
|
||||
|
||||
expect(result.summaryMessage).toBe('Test summary message');
|
||||
expect(result.summaryTitle).toBe('Test Title');
|
||||
expect(result.conversationHistory).toEqual([]);
|
||||
});
|
||||
|
||||
test('should handle missing conversation history', async () => {
|
||||
const inputData = {
|
||||
userName: 'Bob Johnson',
|
||||
messageId: 'msg-789',
|
||||
userId: 'user-789',
|
||||
chatId: 'chat-789',
|
||||
isFollowUp: false,
|
||||
isSlackFollowUp: false,
|
||||
previousMessages: [],
|
||||
datasets: 'test datasets',
|
||||
toolCalled: 'flagChat',
|
||||
flagChatMessage: 'No issues found',
|
||||
flagChatTitle: 'All Clear',
|
||||
assumptions: [
|
||||
{
|
||||
descriptiveTitle: 'Date Range',
|
||||
classification: 'timePeriodInterpretation' as const,
|
||||
explanation: 'Assumed current month',
|
||||
label: 'major' as const
|
||||
}
|
||||
]
|
||||
};
|
||||
|
||||
const result = await formatInitialMessageStepExecution({ inputData });
|
||||
|
||||
expect(result.summaryMessage).toBe('Test summary message');
|
||||
expect(result.summaryTitle).toBe('Test Title');
|
||||
expect(result.conversationHistory).toBeUndefined();
|
||||
});
|
||||
|
||||
test('should return input data unchanged when no major assumptions', async () => {
|
||||
const inputData = {
|
||||
conversationHistory: [
|
||||
{ role: 'user' as const, content: 'Hello' },
|
||||
{ role: 'assistant' as const, content: 'Hi there!' }
|
||||
],
|
||||
userName: 'Alice Cooper',
|
||||
messageId: 'msg-999',
|
||||
userId: 'user-999',
|
||||
chatId: 'chat-999',
|
||||
isFollowUp: false,
|
||||
isSlackFollowUp: false,
|
||||
previousMessages: [],
|
||||
datasets: 'test datasets',
|
||||
toolCalled: 'noIssuesFound',
|
||||
assumptions: [
|
||||
{
|
||||
descriptiveTitle: 'Minor Detail',
|
||||
classification: 'dataFormat' as const,
|
||||
explanation: 'Used standard format',
|
||||
label: 'minor' as const
|
||||
}
|
||||
]
|
||||
};
|
||||
|
||||
const result = await formatInitialMessageStepExecution({ inputData });
|
||||
|
||||
expect(result).toEqual(inputData);
|
||||
expect(result.summaryMessage).toBeUndefined();
|
||||
expect(result.summaryTitle).toBeUndefined();
|
||||
});
|
||||
});
|
Loading…
Reference in New Issue