-
Notifications
You must be signed in to change notification settings - Fork 73
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
ZEPAI-676: Remove fuzzy query in fulltext search to avoid errors with large queries #192
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.
👍 Looks good to me! Reviewed everything up to 33884dd in 10 seconds
More details
- Looked at
15
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:52
- Draft comment:
The variablefuzzy_query
is misleading since the fuzzy query functionality has been removed. Consider renaming it tofulltext_query
orlucene_query
for clarity. This applies to other instances wherefuzzy_query
is used, such as innode_fulltext_search
andcommunity_fulltext_search
. - Reason this comment was not posted:
Confidence changes required:80%
The PR removes the fuzzy query from the fulltext search, but the variable namefuzzy_query
is still used in several places, which can be misleading.
2. graphiti_core/search/search_utils.py:52
- Draft comment:
The variablelucene_query
is a good choice for naming, as it accurately describes the sanitized query. Ensure consistent naming across similar functions. - Reason this comment was not posted:
Confidence changes required:50%
The code is mostly well-structured, but there are some issues with variable naming and DRY principle.
Workflow ID: wflow_n6tfCKpbodaGX0Df
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 62c9746 in 7 seconds
More details
- Looked at
12
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. pyproject.toml:3
- Draft comment:
The version update from 0.3.11 to 0.3.12 is consistent with the PR description. Ensure that this version bump is reflected in any related documentation or release notes. - Reason this comment was not posted:
Confidence changes required:33%
The version update in the pyproject.toml file is consistent with the PR description, which mentions a version bump from 0.3.11 to 0.3.12. This is a minor version update, likely due to the removal of the fuzzy query feature.
2. pyproject.toml:3
- Draft comment:
Ensure that the version number is updated consistently across all relevant files and documentation. - Reason this comment was not posted:
Confidence changes required:33%
The pyproject.toml file does not require a copyright notice, and there are no other issues related to the rules provided.
Workflow ID: wflow_H8WeuiHuZQRuI1uT
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Important
Remove fuzzy query from
fulltext_query()
insearch_utils.py
to prevent errors with large queries.fulltext_query()
insearch_utils.py
.lucene_sanitize(query)
instead oflucene_sanitize(query) + '~'
.fulltext_query()
no longer appends~
to the sanitized query string.pyproject.toml
from0.3.11
to0.3.12
.This description was created by for 62c9746. It will automatically update as commits are pushed.