(EAI-179): Verified answers on server #348

cbush · 2024-02-27T01:19:42Z

Jira: https://jira.mongodb.org/browse/EAI-179

Changes

Implements the ability to return a static answer

TODO

UNIT TESTS
Handle 'hidden' flag
~~Documentation/README updates~~ 🔜 https://jira.mongodb.org/browse/EAI-263
Ensure 'reject query' functionality still works
Return verified answer info (isVerified, date, who verified, etc) to client
Tweak minScore to get reasonable results

packages/mongodb-chatbot-server/src/processors/makeDefaultFindVerifiedAnswer.ts

packages/mongodb-chatbot-server/src/processors/makeVerifiedAnswerGenerateUserPrompt.ts

nlarew · 2024-02-28T20:31:06Z

packages/mongodb-chatbot-server/src/routes/conversations/addMessageToConversation.ts

+    if (metadata) {
+      dataStreamer.streamData({
+        type: "metadata",
+        data: metadata,
+      });
+    }


I think currently we'll only stream a metadata event for verified answers, right? Do we need any conditional handling for delta streaming (immediately below this block) in the case of verified answers?

Or, is the idea that the metadata event is used solely to indicate that any delta events until the next finished event is a verified answer?

I ask because it looks like the metadata payload will include the verified answer text in its entirety so we don't technically need to stream deltas on top of it.

If we were to use that though then I think a type like static with a subtype of VerifiedAnswer might be more direct.

Yeah, I made it flexible because verified answers is only one of many possible 'meta info needed' type of responses.

I should put metadata response below, wasn't as clear exactly where/how to get it though (since the generate message is a bit more complicated). Will look into it.

I included the entire verified answer object for simplicity, but I wouldn't rely on it. I can strip it down to just the essential info.

docs/docs/ingest/configuration-reference.md

mongodben · 2024-02-29T18:29:18Z

packages/mongodb-verified-answers/src/importVerifiedAnswers.ts

is mongodb-verified-answers a package that we should publish? also nit on naming, think it should be mongodb-chatbot-verified-answers for consistency w/ other package names

packages/mongodb-rag-core/src/VerifiedAnswer.ts

packages/mongodb-chatbot-server/src/services/dataStreamer.ts

packages/mongodb-chatbot-server/src/processors/makeDefaultFindVerifiedAnswer.ts

mongodben · 2024-02-29T18:40:10Z

packages/mongodb-chatbot-server/src/processors/makeVerifiedAnswerGenerateUserPrompt.ts

+  continuation: GenerateUserPromptFunc;
+}
+
+export const makeVerifiedAnswerGenerateUserPrompt = ({


think a doc comment on top of the constructor function would be good too.

high level overview of what this GenerateUserPrompt func does. namely:

try to find verified answer for user query

if cannot find, proceed to use the other GenerateUserPrompt func

mongodben · 2024-02-29T18:54:03Z

packages/mongodb-rag-core/src/MongoDbVerifiedAnswerStore.ts

+    connectionUri,
+    databaseName,
+  });
+  const collection = db.collection<VerifiedAnswer>("verified_answers");


can you make the collection name an optional passable param w/ this as the default name?

i think we should move to that as the default pattern for these stores. we can add the same logic to the other stores as well.

mongodben

looks great. nice addition. a few comments throughout, mostly docs related.

if you don't want to include the full documentation on the docs site in this PR b/c you're time limited, we can split it into a separate ticket/PR.

docs/docs/server/verified-answers.md

…ween metadata and customData

mongodben · 2024-03-05T16:06:20Z

packages/chatbot-server-mongodb-public/src/config.ts

+const VERIFIED_ANSWERS_CONNECTION_NAME =
+  process.env.VERIFIED_ANSWERS_CONNECTION_NAME ?? "verified_answers";


Suggested change

const VERIFIED_ANSWERS_CONNECTION_NAME =

process.env.VERIFIED_ANSWERS_CONNECTION_NAME ?? "verified_answers";

const VERIFIED_ANSWERS_COLLECTION_NAME =

process.env.VERIFIED_ANSWERS_COLLECTION_NAME ?? "verified_answers";

i don't think we should do the optionality here since it's our implementation. either it's provided or it isn't and we throw an err.

also per below comment, i think we can include a default collection name in the MongoDBVerifiedAnswerStore.ts file, removing the need for declaring collection name here.

Whoops...

I see what you mean though, we can just not put it in the env then and hardcode it here.

mongodben · 2024-03-05T16:13:50Z

packages/mongodb-rag-core/src/MongoDbVerifiedAnswerStore.ts

+  collectionName,
+}: MakeMongoDbDatabaseConnectionParams & {
+  collectionName: string;


Suggested change

collectionName,

}: MakeMongoDbDatabaseConnectionParams & {

collectionName: string;

collectionName = "verified_answers",

}: MakeMongoDbDatabaseConnectionParams & {

collectionName?: string;

could we do default value here?

One problem with optional arguments: developers forget to pass the variable down and just use the default...

mongodben

lgtm % few small things about collection name in comments

nlarew · 2024-03-05T19:08:53Z

packages/mongodb-chatbot-server/src/routes/conversations/addMessageToConversation.test.ts

Do we need to add tests for the case where this route returns a verified answer?

We effectively already do via existing static response ("no answer found") tests.
For sure though the tests could be improved if we want to set up fake verified_answers to work with in the test database. Can do as a follow-on later.

Verified answers wip

237dcae

mongodben reviewed Feb 27, 2024

View reviewed changes

packages/mongodb-chatbot-server/src/processors/makeDefaultFindVerifiedAnswer.ts Outdated Show resolved Hide resolved

mongodben reviewed Feb 27, 2024

View reviewed changes

packages/mongodb-chatbot-server/src/processors/makeVerifiedAnswerGenerateUserPrompt.ts Show resolved Hide resolved

mongodben reviewed Feb 27, 2024

View reviewed changes

packages/mongodb-chatbot-server/src/processors/makeVerifiedAnswerGenerateUserPrompt.ts Show resolved Hide resolved

cbush added 2 commits February 28, 2024 13:57

Add test

e1b7458

Implement hidden

88a05b9

cbush force-pushed the verified-server branch from deeedf2 to 88a05b9 Compare February 28, 2024 19:13

cbush added 2 commits February 28, 2024 14:25

Update docs

e8de737

Return metadata to client

3e87d36

cbush force-pushed the verified-server branch from ac1a9e3 to 3e87d36 Compare February 28, 2024 19:58

cbush marked this pull request as ready for review February 28, 2024 20:05

nlarew reviewed Feb 28, 2024

View reviewed changes

packages/mongodb-chatbot-server/src/processors/makeVerifiedAnswerGenerateUserPrompt.ts Outdated Show resolved Hide resolved

nlarew reviewed Feb 28, 2024

View reviewed changes

mongodben reviewed Feb 29, 2024

View reviewed changes

docs/docs/ingest/configuration-reference.md Outdated Show resolved Hide resolved

mongodben reviewed Feb 29, 2024

View reviewed changes

docs/docs/ingest/configuration-reference.md Outdated Show resolved Hide resolved

mongodben reviewed Feb 29, 2024

View reviewed changes

packages/mongodb-rag-core/src/VerifiedAnswer.ts Show resolved Hide resolved

mongodben reviewed Feb 29, 2024

View reviewed changes

packages/mongodb-chatbot-server/src/services/dataStreamer.ts Show resolved Hide resolved

mongodben reviewed Feb 29, 2024

View reviewed changes

packages/mongodb-chatbot-server/src/services/dataStreamer.ts Show resolved Hide resolved

mongodben reviewed Feb 29, 2024

View reviewed changes

packages/mongodb-chatbot-server/src/processors/makeDefaultFindVerifiedAnswer.ts Show resolved Hide resolved

mongodben reviewed Feb 29, 2024

View reviewed changes

mongodben requested changes Feb 29, 2024

View reviewed changes

cbush added 6 commits February 29, 2024 15:39

Addressing comments pt 1

9db2b9e

Move mongodb-verified-answers -> mongodb-chatbot-verified-answers

b2b13ee

Addressing comments pt 2

240d883

Merge remote-tracking branch 'upstream/main' into verified-server

1746b32

Fix build

740c65b

Update docs

31ce0ba

cbush requested a review from nlarew February 29, 2024 21:21

cbush requested a review from mongodben February 29, 2024 21:21

mongodben reviewed Feb 29, 2024

View reviewed changes

docs/docs/server/verified-answers.md Outdated Show resolved Hide resolved

mongodben reviewed Feb 29, 2024

View reviewed changes

docs/docs/server/verified-answers.md Outdated Show resolved Hide resolved

mongodben reviewed Feb 29, 2024

View reviewed changes

docs/docs/server/verified-answers.md Outdated Show resolved Hide resolved

Remove docs... for now

21da79a

cbush requested a review from mongodben March 1, 2024 15:20

Return metadata to client, move metadata to base and clarify diff bet…

d8f133e

…ween metadata and customData

mongodben reviewed Mar 5, 2024

View reviewed changes

mongodben approved these changes Mar 5, 2024

View reviewed changes

Remove env for local configs

05c3c32

cbush force-pushed the verified-server branch from 0fb78cf to 05c3c32 Compare March 5, 2024 17:34

nlarew reviewed Mar 5, 2024

View reviewed changes

cbush merged commit e3a9e66 into main Mar 6, 2024
1 check passed

cbush deleted the verified-server branch March 6, 2024 02:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(EAI-179): Verified answers on server #348

(EAI-179): Verified answers on server #348

cbush commented Feb 27, 2024 •

edited

Loading

nlarew Feb 28, 2024 •

edited

Loading

cbush Feb 28, 2024

mongodben Feb 29, 2024

mongodben Feb 29, 2024

mongodben Feb 29, 2024

mongodben left a comment

mongodben Mar 5, 2024

mongodben Mar 5, 2024 •

edited

Loading

mongodben Mar 5, 2024

cbush Mar 5, 2024

mongodben Mar 5, 2024

cbush Mar 5, 2024

mongodben left a comment

nlarew Mar 5, 2024

cbush Mar 6, 2024 •

edited

Loading

		const VERIFIED_ANSWERS_CONNECTION_NAME =
		process.env.VERIFIED_ANSWERS_CONNECTION_NAME ?? "verified_answers";

(EAI-179): Verified answers on server #348

(EAI-179): Verified answers on server #348

Conversation

cbush commented Feb 27, 2024 • edited Loading

Changes

TODO

nlarew Feb 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mongodben left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mongodben Mar 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mongodben left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbush Mar 6, 2024 • edited Loading

Choose a reason for hiding this comment

cbush commented Feb 27, 2024 •

edited

Loading

nlarew Feb 28, 2024 •

edited

Loading

mongodben Mar 5, 2024 •

edited

Loading

cbush Mar 6, 2024 •

edited

Loading