Append and Replace Document Metadata #2054
Merged
Conversation
This file contains hidden or 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
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 0f2aae8 in 3 minutes and 12 seconds
More details
- Looked at
448lines of code in9files - Skipped
2files when reviewing. - Skipped posting
9drafted comments based on config settings.
1. py/sdk/sync_methods/documents.py:102
- Draft comment:
Use a context manager for file opening to ensure proper closure in case of exceptions. For example, usewith open(file_path, 'rb') as file_instance:. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. py/sdk/sync_methods/documents.py:289
- Draft comment:
Verify thatresponse.iter_bytes()exists in the client library. Typically, requests usesiter_content()for streaming binary data. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. py/sdk/asnyc_methods/documents.py:1
- Draft comment:
The directory/file name 'asnyc_methods' appears to be a typo; consider renaming it to 'async_methods' to improve clarity and maintain consistency. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. js/sdk/__tests__/DocumentsIntegrationSuperUser.test.ts:396
- Draft comment:
The test name uses 'non-existant', which appears to be a typographical error. Consider replacing it with 'non-existent' to improve clarity and consistency. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. js/sdk/src/v3/clients/documents.ts:187
- Draft comment:
The JSDoc comment for the 'retrieve' method is misleading. It mentions an optional list of document IDs, offset, and limit, and states that it returns a WrappedDocumentsResponse, while the method signature only accepts a single 'id' parameter and returns a WrappedDocumentResponse. Please update the documentation to accurately reflect the method's parameters and return type. - Reason this comment was not posted:
Comment was on unchanged code.
6. py/sdk/asnyc_methods/documents.py:1
- Draft comment:
The directory name 'asnyc_methods' appears to be a misspelling of 'async_methods'. Please confirm if this is intentional or if it should be corrected. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. py/sdk/asnyc_methods/documents.py:47
- Draft comment:
In the 'create' method docstring, the argument is described as 'content' (line 48) whereas the actual parameter name is 'raw_text'. This inconsistency can be confusing. Please update the documentation to match the parameter name. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. py/sdk/asnyc_methods/documents.py:654
- Draft comment:
In the 'search' method docstring, the parameter description for 'search_settings' has an extra closing bracket: 'Optional[dict, SearchSettings]]'. Please remove the extra ']' to fix the typo. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. py/sdk/sync_methods/documents.py:658
- Draft comment:
Typo: In the search method docstring, the type forsearch_settingsis written as 'Optional[dict, SearchSettings]]'. Consider removing the extra ']' to make it 'Optional[dict, SearchSettings]'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_og5A6ov76Xk9l2uy
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| @self.router.put( | ||
| "/documents/{id}/metadata", | ||
| dependencies=[Depends(self.rate_limit_dependency)], | ||
| summary="Append metadata to a document", |
There was a problem hiding this comment.
The PUT endpoint’s summary still states "Append metadata to a document". Since it calls update_document_metadata with overwrite=True (replacing metadata), update the summary to reflect that this endpoint replaces metadata.
Suggested change
| summary="Append metadata to a document", | |
| summary="Replace metadata of a document", |
There was a problem hiding this comment.
❌ Changes requested. Incremental review on a8a1698 in 2 minutes and 8 seconds
More details
- Looked at
90lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. py/core/main/api/v3/documents_router.py:690
- Draft comment:
The docstring for the PUT endpoint still says 'Appends metadata to a document' while the summary and behavior indicate a replacement. Update the docstring to clarify that metadata is replaced. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. py/core/main/api/v3/documents_router.py:575
- Draft comment:
Good addition of code samples for the PATCH endpoint to append metadata. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
Workflow ID: wflow_OVqfFAjtlWkiqbEF
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
py/core/main/api/v3/documents_router.py
Show resolved
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 264f398 in 41 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. py/core/main/api/v3/documents_router.py:700
- Draft comment:
Docstring updated to reflect metadata replacement behavior. Ensure that this aligns with the underlying service call (overwrite=True) as expected. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
2. py/core/main/api/v3/documents_router.py:691
- Draft comment:
The PUT endpoint docstring now states 'Replaces metadata' but the parameter descriptions (for 'id' and 'metadata') still mention 'append'. Please update these to accurately reflect the replacement behavior. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. py/core/main/api/v3/documents_router.py:691
- Draft comment:
In the put_metadata endpoint, the parameter description for the document ID still refers to 'append metadata' even though the endpoint is now intended to replace metadata. Please update the description to match the intended replace behavior (e.g., "The ID of the document for which metadata is to be replaced."). - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_0weSJHTO3KGj8Vmb
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.
Adds the ability to append and replace the metadata of an existing document via
PATCHandPUTrequests.Closes #2030
Important
Add support for appending and replacing document metadata via
PATCHandPUTrequests, with corresponding client methods and tests.appendMetadata()andreplaceMetadata()methods toDocumentsClientindocuments.tsfor appending and replacing document metadata viaPATCHandPUTrequests.DocumentsRouterindocuments_router.pyto handle/documents/{id}/metadataendpoints for appending and replacing metadata.update_document_metadata()inmanagement_service.pyanddocuments.pyto support metadata operations.DocumentsIntegrationSuperUser.test.ts.package.jsonandpyproject.tomlto increment version numbers.This description was created by
for 264f398. It will automatically update as commits are pushed.