-
Notifications
You must be signed in to change notification settings - Fork 75
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
Use sessions search #197
Use sessions search #197
Conversation
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.
❌ Changes requested. Reviewed everything up to c5541ec in 43 seconds
More details
- Looked at
577
lines of code in5
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. graphiti_core/search/search_utils.py:90
- Draft comment:
Missing comma in the Cypher query. Add a comma aftern.name_embedding AS name_embedding
.
n.name_embedding AS name_embedding,
- Reason this comment was not posted:
Marked as duplicate.
2. graphiti_core/search/search_utils.py:91
- Draft comment:
Missing comma in the Cypher query. Add a comma aftern.name_embedding AS name_embedding
.
n.name_embedding AS name_embedding,
- Reason this comment was not posted:
Marked as duplicate.
3. graphiti_core/search/search_utils.py:116
- Draft comment:
Missing comma in the Cypher query. Add a comma afterc.name_embedding AS name_embedding
.
c.name_embedding AS name_embedding,
- Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_gB92GtwMtjAlKsEM
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.
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.
👍 Looks good to me! Incremental review on 5dd92c8 in 21 seconds
More details
- Looked at
13
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. graphiti_core/search/search_utils.py:87
- Draft comment:
Consider renamingdatabase=DEFAULT_DATABASE
todatabase_=DEFAULT_DATABASE
for consistency with the PR description. This applies to other instances as well (e.g., lines 107, 161, 214, 244, 279, 321, 356). - Reason this comment was not posted:
Confidence changes required:80%
The PR description mentions renaming_database
todatabase_
for consistency, but this change is not reflected in the code. The code still usesdatabase=DEFAULT_DATABASE
in multiple places.
2. graphiti_core/search/search_utils.py:87
- Draft comment:
Consider refactoring the repeated pattern of executing queries and processing results into a separate function to adhere to the DRY principle. This pattern appears in multiple functions such asget_mentioned_nodes
,get_communities_by_nodes
,edge_fulltext_search
, etc. - Reason this comment was not posted:
Confidence changes required:80%
The code contains a repeated pattern for executing queries and processing results. This can be refactored to follow the DRY principle.
Workflow ID: wflow_enASk0B7BaVrvXo2
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Important
Refactor database query execution to use sessions and rename query parameters for consistency across multiple files.
session.run()
for executing queries insearch_utils.py
,community_operations.py
, andgraph_data_operations.py
.execute_query()
withsession.run()
in functions likeget_mentioned_nodes()
,get_communities_by_nodes()
,edge_fulltext_search()
, and others._database
todatabase_
inedges.py
,nodes.py
,search_utils.py
,community_operations.py
, andgraph_data_operations.py
for consistency.This description was created by for 5dd92c8. It will automatically update as commits are pushed.