-
Notifications
You must be signed in to change notification settings - Fork 0
[QUO-898] document metadata support/validation #7
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
Conversation
QUO-898 Add documents metadata to detections endpoint
Allow users to pass metadata about their documents (e.g. as a dictionary). For example - Tavily mentioned they want to report where documents are being retrieved from. |
quotientai/logger.ts
Outdated
| continue; | ||
| } else if (typeof doc === 'object' && doc !== null) { | ||
| if (!this.isValidLogDocument(doc)) { | ||
| throw new ValidationError("Documents must be a list of strings or dictionaries with 'page_content' and optional 'metadata' keys. Metadata keys must be strings"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a user sees this error, what should they do about it?
Whenever we raise an exception we should tell the user what to do in response (as part of the exception message), otherwise they're going to fumble and have to read more docs, read source code, ignore completely, etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some more actionable error msgs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Let’s make sure both messages are in sync in both clients
waldnzwrld
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to update all tests related to logs to include your new type and verify the behaviour you have introduced.
Tests have been added and my initial feedback has been tackled
waldnzwrld
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @crekhari
|
@crekhari would you mind updating the example_logs.ts file for the new documents format as well. 😁 |
Fixed! Ill cut a new release after lunch. Lmk if theres any other changes needed |
Support document metadata