feat(agents-api): Add embeddings to doc get/list response #578
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 70eebc4 in 22 seconds
More details
- Looked at
86lines of code in5files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. agents-api/agents_api/models/docs/get_doc.py:40
- Draft comment:
Consider adding validation for theembeddingsfield to ensure it is of the expected type (list of floats or list of list of floats). This will prevent potential issues if the data does not match the expected type. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is about a change made in the diff, specifically the addition of 'embeddings'. Adding validation for data types can prevent runtime errors and ensure data integrity. However, the comment is speculative as it assumes potential issues without evidence of current problems.
The comment might be unnecessary if the data source guarantees the type of 'embeddings', or if there are existing validations elsewhere in the codebase.
Without evidence of existing validation or guarantees from the data source, the suggestion to add validation is reasonable to prevent future issues.
The comment is relevant to the changes made and suggests a potential improvement in data validation, which is actionable and clear.
2. agents-api/agents_api/models/docs/list_docs.py:37
- Draft comment:
Consider adding validation for theembeddingsfield to ensure it is of the expected type (list of floats or list of list of floats). This will prevent potential issues if the data does not match the expected type. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_cryTiEcOnjlzfuyt
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Important
Add embeddings to document retrieval and listing responses in
Docs.py,get_doc.py,list_docs.py, andmodels.tsp.embeddingsfield toDocmodel inDocs.pyandmodels.tsp.embeddingsin the response ofget_doc()inget_doc.pyandlist_docs()inlist_docs.py.get_queryinget_doc.pyto retrieveembeddingdata.get_queryinlist_docs.pyto includeembeddingdata.Docmodel inmodels.tspto include optionalembeddingsfield withreadvisibility.This description was created by
for 70eebc4. It will automatically update as commits are pushed.