-
Notifications
You must be signed in to change notification settings - Fork 28
feat: refactored ContextGroundingVectorStore to improve type safety #241
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
feat: refactored ContextGroundingVectorStore to improve type safety #241
Conversation
ced5c6f to
189c162
Compare
src/uipath_langchain/vectorstores/context_grounding_vectorstore.py
Outdated
Show resolved
Hide resolved
src/uipath_langchain/vectorstores/context_grounding_vectorstore.py
Outdated
Show resolved
Hide resolved
src/uipath_langchain/vectorstores/context_grounding_vectorstore.py
Outdated
Show resolved
Hide resolved
| execution_context = ExecutionContext() | ||
|
|
||
| # Create a temporary UiPath SDK instance to get the required services | ||
| sdk = UiPath(base_url=self.uipath_url, secret=access_token) |
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.
can t we just initialize UiPath() with the desired base_url and secret and then pass it to the ContextGroundingVectorStore ctor?
why is all this refactor needed?
a7fc34b to
6ab1882
Compare
f3d168d to
365c57e
Compare
| uipath_sdk: Optional UiPath SDK instance. | ||
| folder_path: Optional folder path for folder-scoped operations | ||
| """ | ||
| self.index_name = index_name |
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.
duplicated
| folder_path: Optional folder path for folder-scoped operations | ||
| """ | ||
| self.index_name = index_name | ||
| self.folder_path = folder_path |
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.
same here
b79e226 to
3ea9bf4
Compare
radu-mocanu
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.
add a short PR description with the changes made please
3ea9bf4 to
34e1ed2
Compare
|
commit message should follow conventional commit structure |
34e1ed2 to
95e0385
Compare
Summary
Refactored the ContextGroundingVectorStore implementation to improve type safety, structure, and maintainability while keeping functional parity with the previous version.
⸻
Key Changes
Refactored class for clarity and modern typing
• Added precise type annotations using | unions and Self return types.
• Replaced Optional[...] with more concise modern Python 3.10+ syntax.
• Added @OverRide decorators for all methods implementing the VectorStore interface.
🧩 Introduced _convert_results_to_documents() helper
• Centralized logic for converting ContextGroundingQueryResponse objects to Document instances.
• Improved metadata safety (handles missing or None fields gracefully).
🔐 Reinforced read-only behavior
• add_texts(), delete(), and from_texts() explicitly raise NotImplementedError with clearer messages.