diff --git a/.github/workflows/draft-release.yaml b/.github/workflows/draft-release.yaml index 62ec8fb49..2246bc3ac 100644 --- a/.github/workflows/draft-release.yaml +++ b/.github/workflows/draft-release.yaml @@ -80,7 +80,6 @@ jobs: uses: ./.github/workflows/actions/test-and-build with: SEGMENT_KEY: ${{ secrets.SEGMENT_KEY_PROD }} - MONGODB_DOCS_CHATBOT_BASE_URI: ${{ secrets.MONGODB_DOCS_CHATBOT_BASE_URI_PROD }} ARTIFACTORY_HOST: ${{ secrets.ARTIFACTORY_HOST }} ARTIFACTORY_PASSWORD: ${{ secrets.ARTIFACTORY_PASSWORD }} ARTIFACTORY_USERNAME: ${{ secrets.ARTIFACTORY_USERNAME }} diff --git a/.github/workflows/test-and-build.yaml b/.github/workflows/test-and-build.yaml index 73fc6ef88..ceaaa04f3 100644 --- a/.github/workflows/test-and-build.yaml +++ b/.github/workflows/test-and-build.yaml @@ -36,7 +36,6 @@ jobs: uses: ./.github/workflows/actions/test-and-build with: SEGMENT_KEY: ${{ secrets.SEGMENT_KEY_PROD }} - MONGODB_DOCS_CHATBOT_BASE_URI: ${{ secrets.MONGODB_DOCS_CHATBOT_BASE_URI_PROD }} ARTIFACTORY_HOST: ${{ secrets.ARTIFACTORY_HOST }} ARTIFACTORY_PASSWORD: ${{ secrets.ARTIFACTORY_PASSWORD }} ARTIFACTORY_USERNAME: ${{ secrets.ARTIFACTORY_USERNAME }} diff --git a/scripts/generate-constants.ts b/scripts/generate-constants.ts index 19c738210..e84d60000 100644 --- a/scripts/generate-constants.ts +++ b/scripts/generate-constants.ts @@ -16,14 +16,7 @@ config({ path: resolve(__dirname, '../.env') }); (async () => { await writeFile( `${ROOT_DIR}/constants.json`, - JSON.stringify( - { - segmentKey: process.env.SEGMENT_KEY, - docsChatbotBaseUri: process.env.MONGODB_DOCS_CHATBOT_BASE_URI, - }, - null, - 2 - ) + JSON.stringify({ segmentKey: process.env.SEGMENT_KEY }, null, 2) ); ui.succeed('The constants file has been generated'); })().catch((error) => { diff --git a/src/mdbExtensionController.ts b/src/mdbExtensionController.ts index 98bbf9e7d..a72aee212 100644 --- a/src/mdbExtensionController.ts +++ b/src/mdbExtensionController.ts @@ -155,7 +155,6 @@ export default class MDBExtensionController implements vscode.Disposable { this._telemetryService.activateSegmentAnalytics(); this._participantController.createParticipant(this._context); - await this._participantController.createDocsChatbot(this._context); await this._connectionController.loadSavedConnections(); await this._languageServerController.startLanguageServer(); diff --git a/src/participant/constants.ts b/src/participant/constants.ts index b867f0738..59f80d4f4 100644 --- a/src/participant/constants.ts +++ b/src/participant/constants.ts @@ -16,6 +16,7 @@ export type ParticipantResponseType = interface Metadata { intent: Exclude; chatId: string; + docsChatbotMessageId?: string; } interface AskForNamespaceMetadata { @@ -84,11 +85,18 @@ export function queryRequestChatResult( return createChatResult('query', history); } -export function docsRequestChatResult(chatId: string): ChatResult { +export function docsRequestChatResult({ + chatId, + docsChatbotMessageId, +}: { + chatId: string; + docsChatbotMessageId?: string; +}): ChatResult { return { metadata: { chatId, intent: 'docs', + docsChatbotMessageId, }, }; } diff --git a/src/participant/docsChatbotAIService.ts b/src/participant/docsChatbotAIService.ts index d73de13f5..4b0c8b1b8 100644 --- a/src/participant/docsChatbotAIService.ts +++ b/src/participant/docsChatbotAIService.ts @@ -1,5 +1,7 @@ import type { Reference, VerifiedAnswer } from 'mongodb-rag-core'; +const MONGODB_DOCS_CHATBOT_BASE_URI = 'https://knowledge.mongodb.com/'; + const MONGODB_DOCS_CHATBOT_API_VERSION = 'v1'; // eslint-disable-next-line @typescript-eslint/no-var-requires @@ -40,35 +42,19 @@ type AssistantMessageMetadata = { }; export class DocsChatbotAIService { - _serverBaseUri?: string; + _serverBaseUri: string; - constructor(serverBaseUri?: string) { - this._serverBaseUri = serverBaseUri; - } - - private getServerBaseUri(): string { - if (!this._serverBaseUri) { - throw new Error( - 'You must define a serverBaseUri for the DocsChatbotAIService' - ); - } - return this._serverBaseUri; + constructor() { + this._serverBaseUri = + process.env.MONGODB_DOCS_CHATBOT_BASE_URI_OVERRIDE || + MONGODB_DOCS_CHATBOT_BASE_URI; } private getUri(path: string): string { - const serverBaseUri = this.getServerBaseUri(); - return `${serverBaseUri}api/${MONGODB_DOCS_CHATBOT_API_VERSION}${path}`; - } - - async createConversation(): Promise { - const uri = this.getUri('/conversations'); - return this._fetch({ - uri, - method: 'POST', - }); + return `${this._serverBaseUri}api/${MONGODB_DOCS_CHATBOT_API_VERSION}${path}`; } - async _fetch({ + async _fetch({ uri, method, body, @@ -78,37 +64,49 @@ export class DocsChatbotAIService { method: string; body?: string; headers?: { [key: string]: string }; - }): Promise { - const resp = await fetch(uri, { + }): Promise { + return fetch(uri, { headers: { - origin: this.getServerBaseUri(), + origin: this._serverBaseUri, 'User-Agent': `mongodb-vscode/${version}`, ...headers, }, method, ...(body && { body }), }); - let conversation; + } + + async createConversation(): Promise { + const uri = this.getUri('/conversations'); + const res = await this._fetch({ + uri, + method: 'POST', + }); + + let data; try { - conversation = await resp.json(); + data = await res.json(); } catch (error) { throw new Error('[Docs chatbot] Internal server error'); } - if (resp.status === 400) { - throw new Error(`[Docs chatbot] Bad request: ${conversation.error}`); + if (res.status === 400) { + throw new Error(`[Docs chatbot] Bad request: ${data.error}`); } - if (resp.status === 429) { - throw new Error(`[Docs chatbot] Rate limited: ${conversation.error}`); + if (res.status === 429) { + throw new Error(`[Docs chatbot] Rate limited: ${data.error}`); } - if (resp.status >= 500) { + if (res.status >= 500) { throw new Error( - `[Docs chatbot] Internal server error: ${conversation.error}` + `[Docs chatbot] Internal server error: ${ + data.error ? data.error : `${res.status} - ${res.statusText}}` + }` ); } + return { - ...conversation, - conversationId: conversation._id, + ...data, + conversationId: data._id, }; } @@ -120,11 +118,79 @@ export class DocsChatbotAIService { message: string; }): Promise { const uri = this.getUri(`/conversations/${conversationId}/messages`); - return await this._fetch({ + const res = await this._fetch({ uri, method: 'POST', body: JSON.stringify({ message }), headers: { 'Content-Type': 'application/json' }, }); + + let data; + try { + data = await res.json(); + } catch (error) { + throw new Error('[Docs chatbot] Internal server error'); + } + + if (res.status === 400) { + throw new Error(`[Docs chatbot] Bad request: ${data.error}`); + } + if (res.status === 404) { + throw new Error(`[Docs chatbot] Conversation not found: ${data.error}`); + } + if (res.status === 429) { + throw new Error(`[Docs chatbot] Rate limited: ${data.error}`); + } + if (res.status === 504) { + throw new Error(`[Docs chatbot] Timeout: ${data.error}`); + } + if (res.status >= 500) { + throw new Error( + `[Docs chatbot] Internal server error: ${ + data.error ? data.error : `${res.status} - ${res.statusText}}` + }` + ); + } + + return data; + } + + async rateMessage({ + conversationId, + messageId, + rating, + }: { + conversationId: string; + messageId: string; + rating: boolean; + }): Promise { + const uri = this.getUri( + `/conversations/${conversationId}/messages/${messageId}/rating` + ); + const res = await this._fetch({ + uri, + method: 'POST', + body: JSON.stringify({ rating }), + headers: { 'Content-Type': 'application/json' }, + }); + + if (res.status === 204) { + return rating; + } + + let data; + if (res.status >= 400) { + try { + data = await res.json(); + } catch (error) { + throw new Error(`[Docs chatbot] Internal server error: ${error}`); + } + } + + throw new Error( + `[Docs chatbot] Internal server error: ${ + data.error ? data.error : `${res.status} - ${res.statusText}}` + }` + ); } } diff --git a/src/participant/participant.ts b/src/participant/participant.ts index 4ccd57eff..553737a22 100644 --- a/src/participant/participant.ts +++ b/src/participant/participant.ts @@ -1,9 +1,6 @@ import * as vscode from 'vscode'; import { getSimplifiedSchema } from 'mongodb-schema'; import type { Document } from 'bson'; -import { config } from 'dotenv'; -import path from 'path'; -import { promises as fs } from 'fs'; import { createLogger } from '../logging'; import type ConnectionController from '../connectionController'; @@ -81,7 +78,7 @@ export default class ParticipantController { _connectionController: ConnectionController; _storageController: StorageController; _chatMetadataStore: ChatMetadataStore; - _docsChatbotAIService?: DocsChatbotAIService; + _docsChatbotAIService: DocsChatbotAIService; _telemetryService: TelemetryService; constructor({ @@ -97,37 +94,7 @@ export default class ParticipantController { this._storageController = storageController; this._chatMetadataStore = new ChatMetadataStore(); this._telemetryService = telemetryService; - } - - // To integrate with the MongoDB documentation chatbot, - // set the MONGODB_DOCS_CHATBOT_BASE_URI environment variable when running the extension from a branch. - // This variable is automatically injected during the .vsix build process via GitHub Actions. - async _readDocsChatbotBaseUri( - context: vscode.ExtensionContext - ): Promise { - config({ path: path.join(context.extensionPath, '.env') }); - - try { - const docsChatbotBaseUriFileLocation = path.join( - context.extensionPath, - './constants.json' - ); - // eslint-disable-next-line no-sync - const constantsFile = await fs.readFile( - docsChatbotBaseUriFileLocation, - 'utf8' - ); - const { docsChatbotBaseUri } = JSON.parse(constantsFile) as { - docsChatbotBaseUri?: string; - }; - return docsChatbotBaseUri; - } catch (error) { - log.error( - 'Failed to read docsChatbotBaseUri from the constants file', - error - ); - return; - } + this._docsChatbotAIService = new DocsChatbotAIService(); } createParticipant(context: vscode.ExtensionContext): vscode.ChatParticipant { @@ -143,18 +110,13 @@ export default class ParticipantController { 'images', 'mongodb.png' ); - log.info('Chat Participant Created', { + log.info('Chat participant created', { participantId: this._participant?.id, }); this._participant.onDidReceiveFeedback(this.handleUserFeedback.bind(this)); return this._participant; } - async createDocsChatbot(context: vscode.ExtensionContext): Promise { - const docsChatbotBaseUri = await this._readDocsChatbotBaseUri(context); - this._docsChatbotAIService = new DocsChatbotAIService(docsChatbotBaseUri); - } - getParticipant(): vscode.ChatParticipant | undefined { return this._participant; } @@ -797,35 +759,43 @@ export default class ParticipantController { } async _handleDocsRequestWithChatbot({ - docsChatbotAIService, prompt, chatId, }: { - docsChatbotAIService: DocsChatbotAIService; prompt: string; chatId: string; }): Promise<{ responseContent: string; responseReferences?: Reference[]; + docsChatbotMessageId: string; }> { let { docsChatbotConversationId } = this._chatMetadataStore.getChatMetadata(chatId) ?? {}; if (!docsChatbotConversationId) { - const conversation = await docsChatbotAIService.createConversation(); + const conversation = + await this._docsChatbotAIService.createConversation(); docsChatbotConversationId = conversation._id; this._chatMetadataStore.setChatMetadata(chatId, { docsChatbotConversationId, }); + log.info('Docs chatbot created for chatId', chatId); } - const response = await docsChatbotAIService.addMessage({ + const response = await this._docsChatbotAIService.addMessage({ message: prompt, conversationId: docsChatbotConversationId, }); + log.info('Docs chatbot message sent', { + chatId, + docsChatbotConversationId, + docsChatbotMessageId: response.id, + }); + return { responseContent: response.content, responseReferences: response.references, + docsChatbotMessageId: response.id, }; } @@ -885,29 +855,21 @@ export default class ParticipantController { const chatId = ChatMetadataStore.getChatIdFromHistoryOrNewChatId( context.history ); - - let docsChatbotHasThrownError = false; let docsResult: { responseContent?: string; responseReferences?: Reference[]; + docsChatbotMessageId?: string; } = {}; - if (this._docsChatbotAIService) { - try { - docsResult = await this._handleDocsRequestWithChatbot({ - docsChatbotAIService: this._docsChatbotAIService, - prompt: request.prompt, - chatId, - }); - } catch (error) { - // If the docs chatbot API is not available, fall back to Copilot’s LLM and include - // the MongoDB documentation link for users to go to our documentation site directly. - docsChatbotHasThrownError = true; - log.error(error); - } - } - - if (!this._docsChatbotAIService || docsChatbotHasThrownError) { + try { + docsResult = await this._handleDocsRequestWithChatbot({ + prompt: request.prompt, + chatId, + }); + } catch (error) { + // If the docs chatbot API is not available, fall back to Copilot’s LLM and include + // the MongoDB documentation link for users to go to our documentation site directly. + log.error(error); docsResult = await this._handleDocsRequestWithCopilot(...args); } @@ -929,7 +891,10 @@ export default class ParticipantController { } } - return docsRequestChatResult(chatId); + return docsRequestChatResult({ + chatId, + docsChatbotMessageId: docsResult.docsChatbotMessageId, + }); } async chatHandler( @@ -981,7 +946,39 @@ export default class ParticipantController { return await this.handleGenericRequest(...args); } - handleUserFeedback(feedback: vscode.ChatResultFeedback): void { + async _rateDocsChatbotMessage( + feedback: vscode.ChatResultFeedback + ): Promise { + const chatId = feedback.result.metadata?.chatId; + if (!chatId) { + return; + } + + const { docsChatbotConversationId } = + this._chatMetadataStore.getChatMetadata(chatId) ?? {}; + if ( + !docsChatbotConversationId || + !feedback.result.metadata?.docsChatbotMessageId + ) { + return; + } + + try { + const rating = await this._docsChatbotAIService.rateMessage({ + conversationId: docsChatbotConversationId, + messageId: feedback.result.metadata?.docsChatbotMessageId, + rating: !!feedback.kind, + }); + log.info('Docs chatbot rating sent', rating); + } catch (error) { + log.error(error); + } + } + + async handleUserFeedback(feedback: vscode.ChatResultFeedback): Promise { + if (feedback.result.metadata?.intent === 'docs') { + await this._rateDocsChatbotMessage(feedback); + } this._telemetryService.trackCopilotParticipantFeedback({ feedback: chatResultFeedbackKindToTelemetryValue(feedback.kind), reason: feedback.unhelpfulReason, diff --git a/src/test/suite/participant/docsChatbotAIService.test.ts b/src/test/suite/participant/docsChatbotAIService.test.ts index 6860c7a0b..827cf6841 100644 --- a/src/test/suite/participant/docsChatbotAIService.test.ts +++ b/src/test/suite/participant/docsChatbotAIService.test.ts @@ -6,103 +6,114 @@ import { DocsChatbotAIService } from '../../../participant/docsChatbotAIService' suite('DocsChatbotAIService Test Suite', function () { const initialFetch = global.fetch; + let docsChatbotAIService: DocsChatbotAIService; + + beforeEach(() => { + docsChatbotAIService = new DocsChatbotAIService(); + }); afterEach(function () { global.fetch = initialFetch; sinon.restore(); }); - suite('when serverBaseUri is missing', function () { - test('DocsChatbotAIService constructor does not throw', () => { - const docsChatbotAIService = new DocsChatbotAIService(); - expect(docsChatbotAIService._serverBaseUri).to.be.undefined; - }); - - test('createConversation throws if serverBaseUri is not set', async () => { - const docsChatbotAIService = new DocsChatbotAIService(); - try { - await docsChatbotAIService.createConversation(); - expect.fail('It must fail with missing serverBaseUri'); - } catch (error) { - expect((error as Error).message).to.include( - 'You must define a serverBaseUri for the DocsChatbotAIService' - ); - } + test('creates conversations', async () => { + const fetchStub = sinon.stub().resolves({ + status: 200, + ok: true, + json: () => + Promise.resolve({ + _id: '650b4b260f975ef031016c8a', + messages: [], + }), }); + global.fetch = fetchStub; + const conversation = await docsChatbotAIService.createConversation(); + expect(conversation._id).to.be.eql('650b4b260f975ef031016c8a'); }); - suite('when serverBaseUri is present', function () { - const serverBaseUri = 'https://example.com/'; - let docsChatbotAIService: DocsChatbotAIService; - - beforeEach(() => { - docsChatbotAIService = new DocsChatbotAIService(serverBaseUri); + test('throws on server errors', async () => { + const fetchStub = sinon.stub().resolves({ + status: 500, + ok: false, + statusText: 'Server error', + json: sinon.stub().rejects(new Error('invalid json')), }); + global.fetch = fetchStub; - test('creates conversations', async () => { - const fetchStub = sinon.stub().resolves({ - status: 200, - ok: true, - json: () => - Promise.resolve({ - _id: '650b4b260f975ef031016c8a', - messages: [], - }), - }); - global.fetch = fetchStub; - const conversation = await docsChatbotAIService.createConversation(); - expect(conversation._id).to.be.eql('650b4b260f975ef031016c8a'); + try { + await docsChatbotAIService.createConversation(); + expect.fail('It must fail with the server error'); + } catch (error) { + expect((error as Error).message).to.include('Internal server error'); + } + }); + + test('throws on bad requests', async () => { + const fetchStub = sinon.stub().resolves({ + status: 400, + ok: false, + statusText: 'Client error', + json: sinon.stub().resolves({}), }); + global.fetch = fetchStub; - test('throws on server errors', async () => { - const fetchStub = sinon.stub().resolves({ - status: 500, - ok: false, - statusText: 'Server error', - json: sinon.stub().rejects(new Error('invalid json')), - }); - global.fetch = fetchStub; + try { + await docsChatbotAIService.createConversation(); + expect.fail('It must fail with the bad request error'); + } catch (error) { + expect((error as Error).message).to.include('Bad request'); + } + }); - try { - await docsChatbotAIService.createConversation(); - expect.fail('It must fail with the server error'); - } catch (error) { - expect((error as Error).message).to.include('Internal server error'); - } + test('throws on a rate limit', async () => { + const fetchStub = sinon.stub().resolves({ + status: 429, + ok: false, + statusText: 'Model error', + json: sinon.stub().resolves({}), }); + global.fetch = fetchStub; - test('throws on bad requests', async () => { - const fetchStub = sinon.stub().resolves({ - status: 400, - ok: false, - statusText: 'Client error', - json: sinon.stub().resolves({}), - }); - global.fetch = fetchStub; + try { + await docsChatbotAIService.createConversation(); + expect.fail('It must fail with the rate limited error'); + } catch (error) { + expect((error as Error).message).to.include('Rate limited'); + } + }); - try { - await docsChatbotAIService.createConversation(); - expect.fail('It must fail with the bad request error'); - } catch (error) { - expect((error as Error).message).to.include('Bad request'); - } + test('throws on timeout', async () => { + const fetchStub = sinon.stub().resolves({ + status: 504, + ok: false, + json: sinon.stub().resolves({}), }); + global.fetch = fetchStub; - test('throws on a rate limit', async () => { - const fetchStub = sinon.stub().resolves({ - status: 429, - ok: false, - statusText: 'Model error', - json: sinon.stub().resolves({}), + try { + await docsChatbotAIService.addMessage({ + conversationId: '650b4b260f975ef031016c8a', + message: 'what is mongosh?', }); - global.fetch = fetchStub; + expect.fail('It must fail with the timeout error'); + } catch (error) { + expect((error as Error).message).to.include('Timeout'); + } + }); - try { - await docsChatbotAIService.createConversation(); - expect.fail('It must fail with the rate limited error'); - } catch (error) { - expect((error as Error).message).to.include('Rate limited'); - } + test('rates docs chatbot response', async () => { + const fetchStub = sinon.stub().resolves({ + status: 204, + ok: true, + json: () => Promise.resolve(true), + }); + global.fetch = fetchStub; + const rating = await docsChatbotAIService.rateMessage({ + conversationId: '650b4b260f975ef031016c8a', + messageId: '1', + rating: true, }); + expect(rating).to.be.eql(true); }); }); diff --git a/src/test/suite/participant/participant.test.ts b/src/test/suite/participant/participant.test.ts index 354a95a95..9f13bd2a1 100644 --- a/src/test/suite/participant/participant.test.ts +++ b/src/test/suite/participant/participant.test.ts @@ -1060,12 +1060,18 @@ suite('Participant Controller Test Suite', function () { const initialFetch = global.fetch; let fetchStub; + beforeEach(function () { + sendRequestStub.onCall(0).resolves({ + text: ['connection info'], + }); + }); + afterEach(function () { global.fetch = initialFetch; sinon.restore(); }); - beforeEach(function () { + test('uses docs chatbot result if available', async function () { fetchStub = sinon.stub().resolves({ status: 200, ok: true, @@ -1076,20 +1082,6 @@ suite('Participant Controller Test Suite', function () { }), }); global.fetch = fetchStub; - sendRequestStub.onCall(0).resolves({ - text: ['connection info'], - }); - }); - - test('uses docs chatbot when it is available', async function () { - sinon.replace( - testParticipantController, - '_readDocsChatbotBaseUri', - sinon.stub().resolves('url') - ); - await testParticipantController.createDocsChatbot( - extensionContextStub - ); const chatRequestMock = { prompt: 'how to connect to mongodb', command: 'docs', @@ -1100,14 +1092,20 @@ suite('Participant Controller Test Suite', function () { expect(sendRequestStub).to.have.not.been.called; }); - test('falls back to the copilot model when docs chatbot api is not available', async function () { + test('falls back to the copilot model when docs chatbot result is not available', async function () { + fetchStub = sinon.stub().resolves({ + status: 500, + ok: false, + statusText: 'Internal Server Error', + json: () => Promise.reject(new Error('invalid json')), + }); + global.fetch = fetchStub; const chatRequestMock = { prompt: 'how to connect to mongodb', command: 'docs', references: [], }; await invokeChatHandler(chatRequestMock); - expect(fetchStub).to.have.not.been.called; expect(sendRequestStub).to.have.been.called; }); }); @@ -1115,8 +1113,8 @@ suite('Participant Controller Test Suite', function () { }); suite('telemetry', function () { - test('reports positive user feedback', function () { - testParticipantController.handleUserFeedback({ + test('reports positive user feedback', async function () { + await testParticipantController.handleUserFeedback({ kind: vscode.ChatResultFeedbackKind.Helpful, result: { metadata: { @@ -1142,8 +1140,8 @@ suite('Participant Controller Test Suite', function () { .and.not.include('1234-5678-9012-3456'); }); - test('reports negative user feedback', function () { - testParticipantController.handleUserFeedback({ + test('reports negative user feedback', async function () { + await testParticipantController.handleUserFeedback({ kind: vscode.ChatResultFeedbackKind.Unhelpful, result: { metadata: {