Skip to content

(EAI-923) universal tagging system #671

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

(EAI-923) universal tagging system #671

wants to merge 8 commits into from

Conversation

yakubova92
Copy link
Collaborator

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

Changes

  • mongoDbMetada moved to mongodb-rag-core
  • MongoDbTags enum created
  • validateTags function created - checks list of tags against MongoDbTags enum
  • generateEvalCasesYamlFromCSV accepts list of tags and uses validateTags function

Notes

@yakubova92 yakubova92 requested review from nlarew and mongodben April 7, 2025 18:50
/**
All possible MongoDB tags as enum.
*/
export const mongoDbTags = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm confused by this. isn't this a record, not an enum?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typescript does support enums as a firstclass construct https://www.w3schools.com/typescript/typescript_enums.php

consider also doing a Set

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, its not an enum. I made some changes that simplify it a bit.

Copy link
Collaborator

@mongodben mongodben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally looks good. see comment about the mongoDbTags datastructure

@@ -25,3 +25,4 @@ export * from "./References";
export * from "./VectorStore";
export * from "./arrayFilters";
export * from "./assertEnvVars";
export * from "./mongoDbMetadata";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export * from "./mongoDbMetadata";

pls do this as a named export. in the PR #678, i added much of the metadata stuff.

it's exported as mongodb-rag-core/mongoDbMetadata.

pls update the imports throught this PR accordingly.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

export this from mongoDbMetadata/index.ts

Copy link
Collaborator

@mongodben mongodben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM % note about exporting from mongodb-rag-core/mongoDbMetdata

lmk if any questions on what i mean by this/how to do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants