Skip to content
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

node label filters #265

Merged
merged 7 commits into from
Feb 21, 2025
Merged

node label filters #265

merged 7 commits into from
Feb 21, 2025

Conversation

prasmussen15
Copy link
Collaborator

@prasmussen15 prasmussen15 commented Feb 20, 2025

Important

Add node label filtering to search functionality, updating SearchFilters and related search functions, with tests and version increment to 0.7.1.

  • Behavior:
    • Add node_labels to SearchFilters in search_filters.py for filtering nodes by labels.
    • Update get_relevant_nodes() in graphiti.py and bulk_utils.py to use SearchFilters with node_labels.
    • Modify node_fulltext_search(), node_similarity_search(), and node_bfs_search() in search_utils.py to apply node_labels filter.
  • Functions:
    • Rename search_filter_query_constructor() to node_search_filter_query_constructor() and edge_search_filter_query_constructor() in search_filters.py.
    • Update search() in search.py to pass SearchFilters with node_labels to search functions.
  • Tests:
    • Update tests in search_utils_test.py to include SearchFilters with node_labels in hybrid_node_search() calls.
  • Misc:
    • Increment version to 0.7.1 in pyproject.toml.

This description was created by Ellipsis for 0270abf. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 b8975fb in 2 minutes and 30 seconds

More details
  • Looked at 221 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 12 drafted comments based on config settings.
1. graphiti_core/search/search.py:68
  • Draft comment:
    Good: Passing search_filter to all search functions. Verify that all downstream calls properly handle the new parameter.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 80%
    None
2. graphiti_core/search/search_filters.py:57
  • Draft comment:
    Consider sanitizing or validating node_labels to avoid issues with special characters in labels.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 80%
    None
3. graphiti_core/search/search_filters.py:65
  • Draft comment:
    edge_search_filter_query_constructor is currently empty. Consider implementing filtering logic similar to node search if needed.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
4. graphiti_core/search/search_utils.py:224
  • Draft comment:
    In edge_similarity_search, parameter names in the group filter query do not match the ones set (e.g. using $source_uuid instead of $source_node_uuid). Please ensure the placeholders match the keys in query_params.
  • 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. graphiti_core/search/search_utils.py:218
  • Draft comment:
    In the edge_similarity_search function, the query uses parameters $source_uuid and $target_uuid while the code sets 'source_node_uuid' and 'target_node_uuid' in query_params. Please use consistent parameter names (e.g. use source_uuid/target_uuid everywhere) to avoid potential mismatches.
  • 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.
6. graphiti_core/search/search_utils.py:449
  • Draft comment:
    In node_bfs_search, the WHERE clause uses '$origin.group_id' but 'origin' is an alias from the MATCH clause—not a parameter. Replace '$origin.group_id' with 'origin.group_id' for correct referencing.
  • Reason this comment was not posted:
    Marked as duplicate.
7. graphiti_core/search/search_filters.py:57
  • Draft comment:
    In node_search_filter_query_constructor, consider prepending a space before the 'AND n:' fragment (e.g. ' AND n:…') to ensure proper spacing when concatenated with preceding query parts. Also, validate or sanitize the node_labels to prevent potential injection risks if these values originate from user input.
  • Reason this comment was not posted:
    Confidence changes required: 80% <= threshold 80%
    None
8. graphiti_core/search/search_filters.py:84
  • Draft comment:
    Typo: In the loop that builds the AND filters for valid_at, the conditional 'if j != len(and_filter_query) - 1:' mistakenly uses the length of the built query string instead of the number of filters. This should likely be 'if j != len(and_filters) - 1:' to correctly insert the separator between filter conditions.
  • 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. graphiti_core/search/search_filters.py:109
  • Draft comment:
    Typo: In the invalid_at filters block, the check 'if j != len(and_filter_query) - 1:' incorrectly uses 'and_filter_query' instead of 'and_filters'. Changing this to 'if j != len(and_filters) - 1:' will ensure the correct placement of 'AND' between filter elements.
  • 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.
10. graphiti_core/search/search_filters.py:134
  • Draft comment:
    Typo: In the created_at block, the condition 'if j != len(and_filter_query) - 1:' should use the length of 'and_filters' rather than 'and_filter_query'. Updating this to 'if j != len(and_filters) - 1:' will correctly insert 'AND' between the filter items.
  • 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.
11. graphiti_core/search/search_filters.py:159
  • Draft comment:
    Typo: Within the expired_at filters loop, the 'if j != len(and_filter_query) - 1:' is likely intended to compare 'j' with the number of filters in 'and_filters'. Changing it to 'if j != len(and_filters) - 1:' will ensure the correct insertion of 'AND'.
  • 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.
12. graphiti_core/search/search_utils.py:129
  • Draft comment:
    There's a typographical error in the community query's RETURN clause: after 'c.name_embedding AS name_embedding', a comma is missing before the next field 'c.created_at AS created_at,'. Please add the comma to ensure the query is formed correctly.
  • 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_eNPKWp3Yt8FQhK4F


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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 dd4ec58 in 2 minutes and 37 seconds

More details
  • Looked at 113 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 16 drafted comments based on config settings.
1. graphiti_core/graphiti.py:355
  • Draft comment:
    Passing a default SearchFilters() now to get_relevant_nodes is clear. Consider making this a consistent pattern throughout to avoid repeating SearchFilters() everywhere (e.g., setting a default in get_relevant_nodes if none is provided).
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 80% vs. threshold = 80%
    The comment identifies a valid DRY (Don't Repeat Yourself) issue. SearchFilters() is passed multiple times with the same default value. Making it a default parameter would reduce duplication and improve maintainability. The suggestion is specific and actionable. However, I should consider if there could be cases where we want different filter values.
    What if there are cases where we want to pass different SearchFilters? Making it a default might hide that flexibility. Also, is this really important enough to warrant a code change?
    The code shows SearchFilters() being passed with empty/default values consistently. Even if we need different filters sometimes, having a default doesn't prevent passing custom filters when needed. The change would make the code cleaner.
    Keep the comment. It suggests a valid improvement that would reduce code duplication while maintaining flexibility, aligning with DRY principles.
2. graphiti_core/graphiti.py:738
  • Draft comment:
    Using SearchFilters() when calling get_relevant_nodes in add_triplet ensures consistency. Consider refactoring to avoid hard-coding defaults repeatedly.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
3. graphiti_core/search/search.py:151
  • Draft comment:
    Properly passing 'search_filter' to edge_fulltext_search improves configurability. Ensure similar changes are propagated for all search functions.
  • 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. graphiti_core/search/search.py:250
  • Draft comment:
    Including 'search_filter' in node_similarity_search call is good; make sure all search function signatures and calls remain consistent with the new parameter.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 80%
    This comment is asking the author to ensure consistency in function signatures and calls, which is a form of asking them to double-check their work. This violates the rule against asking the author to confirm or ensure things. It doesn't provide a specific suggestion or point out a specific issue.
5. graphiti_core/search/search.py:253
  • Draft comment:
    The call to node_bfs_search now correctly passes search_filter; ensure that the search_filter parameter is documented in the function docstrings.
  • 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.
6. graphiti_core/search/search_utils.py:449
  • Draft comment:
    Changing the BFS query from using '$origin.group_id' to 'origin.group_id' correctly uses the node alias. This fixes a potential bug in matching group IDs.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
7. graphiti_core/search/search_utils.py:613
  • Draft comment:
    In hybrid_node_search, passing search_filter to both node_fulltext_search and node_similarity_search promotes code reuse. Consider documenting the parameter expectation.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 80% vs. threshold = 80%
    The comment points out a real documentation gap - the search_filter parameter is not documented in hybrid_node_search's docstring despite being passed to other functions. However, the docstring already has good documentation for other parameters. The comment is specific and actionable - it's clear what needs to be added. This would help developers understand the expected structure/format of search_filter.
    The comment could be seen as too minor since search_filter is a typed parameter (SearchFilters) and its usage is clear from the code. The docstring is already quite comprehensive.
    While the type hint provides some information, documenting parameter expectations in docstrings is still valuable, especially for complex filter objects that affect behavior. The comment identifies a specific documentation improvement.
    Keep the comment as it suggests a specific, actionable documentation improvement that would help code maintainability.
8. graphiti_core/search/search_utils.py:638
  • Draft comment:
    The get_relevant_nodes function signature now requires search_filter. Ensure that its updated behavior is clearly documented in the docstring.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 80%
    The comment is asking the PR author to ensure that the updated behavior is documented in the docstring. This falls under the rule of not asking the author to ensure behavior is documented or tested. Therefore, this comment should be removed.
9. graphiti_core/graphiti.py:355
  • Draft comment:
    Inline creation of a default SearchFilters() instance is used here. Consider assigning it to a variable if the same filter is used repeatedly for clarity and maintainability.
  • Reason this comment was not posted:
    Confidence changes required: 66% <= threshold 80%
    None
10. graphiti_core/graphiti.py:737
  • Draft comment:
    Passing SearchFilters() inline here; consider reusing a default instance to avoid redundancy if the same filter is desired.
  • Reason this comment was not posted:
    Confidence changes required: 66% <= threshold 80%
    None
11. graphiti_core/search/search.py:247
  • Draft comment:
    Search helper functions in node_search now consistently receive the search_filter parameter. Verify that downstream functions are updated accordingly.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 80%
    This comment is asking the PR author to verify that downstream functions are updated, which violates the rule against asking the author to confirm or ensure things. It doesn't provide a specific suggestion or point out a specific issue.
12. graphiti_core/search/search.py:565
  • Draft comment:
    Hybrid node search now correctly passes the search_filter parameter. Ensure that the filter logic in the underlying functions handles this parameter as intended.
  • 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.
13. graphiti_core/search/search_utils.py:449
  • Draft comment:
    Bug fix: Changed 'WHERE n.group_id = $origin.group_id' to 'WHERE n.group_id = origin.group_id' in node_bfs_search. This ensures the correct variable is used for comparison.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 80%
    This comment is purely informative, explaining a change that was made. It doesn't ask for any specific action or provide a suggestion for improvement. According to the rules, purely informative comments should be removed.
14. graphiti_core/search/search_utils.py:638
  • Draft comment:
    The signature for get_relevant_nodes has been updated to include search_filter. Ensure all callers pass an appropriate filter to maintain consistent filtering behavior.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 80%
    This comment is asking the PR author to ensure that all callers pass an appropriate filter, which violates the rule against asking the author to ensure behavior is intended or tested. It doesn't provide a specific suggestion or point out a specific issue.
15. graphiti_core/search/search_utils.py:129
  • Draft comment:
    Typographical error: In the Cypher query for get_communities_by_nodes, a comma is missing between c.name_embedding AS name_embedding and c.created_at AS created_at. This might lead to syntax issues.
  • 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.
16. graphiti_core/search/search_utils.py:745
  • Draft comment:
    Typographical error: In the node_distance_reranker function, the query contains 'MATCH p = SHORTEST 1 (...)'. The '1' appears to be a typo and should likely be 'PATH' (i.e., 'MATCH p = SHORTEST PATH ...').
  • 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_2FyDqP6WpNHx7Ov7


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 f1c15c7 in 56 seconds

More details
  • Looked at 36 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 drafted comments based on config settings.
1. graphiti_core/search/search.py:262
  • Draft comment:
    Ensure that the node_bfs_search function signature now accepts the new search_filter parameter. This change is consistent with edge_bfs_search; verify that all related functions are updated accordingly.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 50% <= threshold 80%
    This comment is asking the author to ensure that related functions are updated, which violates the rule against asking the author to double-check things. However, it also provides a specific suggestion about the function signature change, which is useful.
2. graphiti_core/utils/bulk_utils.py:192
  • Draft comment:
    Passing a default SearchFilters() instance to get_relevant_nodes. Confirm that using a default filter here is intentional and meets use case requirements.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 30% <= threshold 80%
    The comment is asking the PR author to confirm their intention, which violates the rule against asking for confirmation. However, it does point out a specific code behavior that might be worth reviewing, which is the use of a default instance of SearchFilters. The comment could be rephrased to suggest a review of the default behavior without asking for confirmation.
3. graphiti_core/search/search.py:261
  • Draft comment:
    Added 'search_filter' as an argument to node_bfs_search for consistency with other BFS calls. Ensure the node_bfs_search signature is updated to accept this extra parameter.
  • 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. graphiti_core/utils/bulk_utils.py:192
  • Draft comment:
    Updated get_relevant_nodes call to pass a default SearchFilters instance. Confirm that using SearchFilters() as the default is appropriate in all cases.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 60% <= threshold 80%
    The comment is asking the PR author to confirm if using a default instance of SearchFilters is appropriate in all cases. This violates the rule against asking the author to confirm their intention or ensure behavior is intended. However, it does point out a specific change that might need attention, which is useful. I need to decide if the comment is more about asking for confirmation or pointing out a potential issue.
5. graphiti_core/search/search.py:350
  • Draft comment:
    Typographical issue: In the community_search function's cross_encoder branch, the dictionary comprehension uses 'node' as the loop variable (i.e., {node.summary: node.uuid ...}) even though the items represent CommunityNode objects. For clarity and consistency, please consider renaming 'node' to 'community'.
  • 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_IGTXBfoaEaTJsLwY


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 99d591b in 1 minute and 50 seconds

More details
  • Looked at 52 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 13 drafted comments based on config settings.
1. graphiti_core/search/search_filters.py:59
  • Draft comment:
    Good fix adding a leading space for consistency in the node label filter.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 80%
    None
2. graphiti_core/search/search_filters.py:72
  • Draft comment:
    Consistent whitespace added for valid_at filter query. Looks better.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 80%
    None
3. graphiti_core/search/search_filters.py:97
  • Draft comment:
    Consistent whitespace added for invalid_at filter query.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 80%
    None
4. graphiti_core/search/search_filters.py:122
  • Draft comment:
    Consistent whitespace added for created_at filter query.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 80%
    None
5. graphiti_core/search/search_filters.py:147
  • Draft comment:
    Consider reviewing the expired_at filter: unlike others, it lacks a leading space ('AND (' vs ' AND ('). Ensure 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.
6. tests/test_graphiti_int.py:32
  • Draft comment:
    Added import for SearchFilters; this appears correct for test dependencies.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 80%
    None
7. graphiti_core/search/search_filters.py:59
  • Draft comment:
    Leading whitespace added to 'AND n:' for consistency. Confirm intended query concatenation.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 80% <= threshold 80%
    The comment is asking for confirmation about the intention behind adding leading whitespace to a query string. This is a specific suggestion related to code style and consistency, which is generally useful feedback. It doesn't violate any of the rules provided.
8. graphiti_core/search/search_filters.py:72
  • Draft comment:
    Prefixed space added in valid_at filter string. Ensure this formatting is consistent with overall query building.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 80% <= threshold 80%
    The comment is pointing out a potential inconsistency in formatting, which is important for maintaining code readability and consistency. It does not ask the author to confirm their intention or to ensure behavior, but rather to ensure consistency in formatting, which is a valid concern.
9. graphiti_core/search/search_filters.py:97
  • Draft comment:
    Prefixed space added in invalid_at filter string; check consistency with similar query fragments.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 80% <= threshold 80%
    The comment points out a potential inconsistency in the query fragment, which could lead to unexpected behavior or bugs. It suggests checking for consistency, which is a valid concern in code review. However, it doesn't directly ask the author to confirm their intention or to ensure the behavior is intended, which would violate the rules. Instead, it suggests a specific action: checking consistency with similar query fragments. This aligns with the rules for making code suggestions.
10. graphiti_core/search/search_filters.py:122
  • Draft comment:
    Prefixed space added in created_at filter string; verify consistency in filter query formatting.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 30% <= threshold 80%
    The comment is asking to verify consistency in filter query formatting, which is against the rule of not asking the PR author to verify or ensure things. However, it does point out a specific issue with the formatting, which could be useful if rephrased to suggest a change rather than asking for verification.
11. tests/test_graphiti_int.py:32
  • Draft comment:
    Imported SearchFilters is not used in this file; remove unused imports to keep the code clean.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 80%
    While the comment is technically correct that SearchFilters is unused, removing unused imports is something that most IDEs and linters would catch automatically. According to the rules, we shouldn't comment on things that would be obviously caught by the build, and unused imports fall into this category.
    The comment is factually accurate. Maybe the project doesn't have linting set up properly and this kind of feedback is actually valuable?
    Even if linting isn't set up, commenting on unused imports is too minor of an issue to warrant a PR comment. This is basic code hygiene that should be handled by automated tools.
    Delete this comment as it points out an issue that should be caught by automated tools rather than manual review.
12. graphiti_core/search/search_filters.py:147
  • Draft comment:
    Typo: The string for 'expired_at_filter' is initialized as 'AND (' without a leading space, while other filters use ' AND ('. For consistency and to ensure the query is constructed as expected, consider updating it to ' AND ('.
  • 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.
13. tests/test_graphiti_int.py:41
  • Draft comment:
    Typographical inconsistency: The variables 'NEO4j_USER' and 'NEO4j_PASSWORD' (lines 41-42) use a lowercase 'j', whereas 'NEO4J_URI' is fully uppercase. For clarity and consistency, consider renaming them to 'NEO4J_USER' and 'NEO4J_PASSWORD'.
  • 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_kk1tDVsaW1LFyIJ0


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 930e436 in 56 seconds

More details
  • Looked at 85 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 7 drafted comments based on config settings.
1. pyproject.toml:3
  • Draft comment:
    Version updated from 0.7.0 to 0.7.1. Ensure that the version bump is consistent with changes in this release.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 80%
    None
2. tests/utils/search/search_utils_test.py:34
  • Draft comment:
    Passing a new SearchFilters() instance is consistent. Confirm that SearchFilters is properly configured for all tests.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 80%
    None
3. tests/utils/search/search_utils_test.py:115
  • Draft comment:
    Test 'with_limit' appropriately validates that the limit passed per search method is doubled. Verify this expected behavior is documented in the function's API.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 80%
    None
4. tests/utils/search/search_utils_test.py:151
  • Draft comment:
    Test 'with_limit_and_duplicates' correctly handles deduplication and limit multiplication. Confirm that the multiplications of limit (2x) are in line with implementation.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 80%
    None
5. tests/utils/search/search_utils_test.py:1
  • Draft comment:
    Missing copyright header. All Python files should start with the appropriate copyright/license notice.
  • 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.
6. tests/utils/search/search_utils_test.py:10
  • Draft comment:
    Repeated instantiation of SearchFilters() in multiple tests. Consider using a fixture to avoid duplication.
  • Reason this comment was not posted:
    Confidence changes required: 66% <= threshold 80%
    None
7. tests/test_graphiti_int.py:40
  • Draft comment:
    Typographical inconsistency: The environment variables NEO4j_USER and NEO4j_PASSWORD (lines 40-41) use a lowercase 'j', which is inconsistent with NEO4J_URI. Consider renaming them to NEO4J_USER and NEO4J_PASSWORD for 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.

Workflow ID: wflow_ty6n2guz5Fvreeeq


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 7a7fd3c in 1 minute and 15 seconds

More details
  • Looked at 27 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 6 drafted comments based on config settings.
1. tests/utils/search/search_utils_test.py:1
  • Draft comment:
    Add copyright header at the top of the file.
  • 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. tests/utils/search/search_utils_test.py:126
  • Draft comment:
    Using SearchFilters() as argument instead of a query string appears intentional; please ensure the function signature is updated accordingly.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 80%
    None
3. tests/utils/search/search_utils_test.py:165
  • Draft comment:
    Verify that passing SearchFilters() maintains consistency in function parameters; ensure all tests now use the updated argument order.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 80%
    None
4. tests/utils/search/search_utils_test.py:1
  • Draft comment:
    Please add the required copyright header at the top of this Python file.
  • 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. tests/utils/search/search_utils_test.py:126
  • Draft comment:
    Updated assert calls now include an explicit SearchFilters() parameter. Ensure that the SearchFilters class implements eq so that the instance compares correctly in these assertions. Consider reusing a default filters instance to reduce redundancy.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 80%
    None
6. tests/utils/search/search_utils_test.py:165
  • Draft comment:
    Similarly, the assertions in test_hybrid_node_search_with_limit_and_duplicates now include a SearchFilters() parameter. Verify that SearchFilters objects compare as expected and consider DRY improvements if this pattern repeats.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 80%
    None

Workflow ID: wflow_0gKrev67GNLRzmuH


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 0270abf in 1 minute and 37 seconds

More details
  • Looked at 71 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 9 drafted comments based on config settings.
1. tests/utils/search/search_utils_test.py:17
  • Draft comment:
    Extra indent before the patch string is inconsistent with surrounding code. Please check formatting for consistency with PEP8.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 80%
    None
2. tests/utils/search/search_utils_test.py:126
  • Draft comment:
    The argument order for node_fulltext_search in assert_called_with has been changed to (driver, 'Test', SearchFilters(), ...). Ensure that this new order matches the actual function signature.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 80%
    This comment is asking the PR author to ensure that the argument order matches the function signature, which violates the rule against asking the author to confirm or ensure things. It doesn't provide a specific suggestion or point out a clear issue.
3. tests/utils/search/search_utils_test.py:128
  • Draft comment:
    The argument order for node_similarity_search in assert_called_with now appears as (driver, [embedding], SearchFilters(), ...) instead of (driver, SearchFilters(), [embedding], ...). Verify that this order is correct as per the function definition.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 80%
    This comment is asking the PR author to verify the order of arguments in a function call, which violates the rule against asking the author to confirm their intention or to double-check things. It does not provide a specific suggestion or point out a clear issue with the code.
4. tests/utils/search/search_utils_test.py:1
  • Draft comment:
    Missing header with copyright notice. Please add the license header at the top of this Python file.
  • 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. tests/utils/search/search_utils_test.py:17
  • Draft comment:
    Inconsistent indentation in the patch invocation (node_fulltext_search) – ensure the indentation is uniform throughout the file.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 80%
    None
6. tests/utils/search/search_utils_test.py:126
  • Draft comment:
    Argument order in assert_called_with for node_fulltext_search has been updated. Ensure the order (driver, query, SearchFilters(), groups, limit) matches the function signature.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 80%
    This comment is asking the author to ensure that the argument order matches the function signature. It is not suggesting a specific change or improvement, but rather asking for confirmation. This violates the rule against asking the author to confirm their intention or ensure behavior is intended.
7. tests/utils/search/search_utils_test.py:128
  • Draft comment:
    Updated argument order in assert_called_with for node_similarity_search. Verify it now follows (driver, embedding, SearchFilters(), groups, limit).
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 40% <= threshold 80%
    The comment is asking the PR author to verify the order of arguments in a function call. This is against the rule of not asking the author to verify or confirm things. However, it does provide specific information about the expected order, which could be useful for the author to check against their changes.
8. tests/utils/search/search_utils_test.py:165
  • Draft comment:
    Ensure the assert_called_with for node_fulltext_search in the duplicate test uses the correct updated argument order (driver, query, SearchFilters(), groups, limit).
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
9. tests/utils/search/search_utils_test.py:167
  • Draft comment:
    Ensure the assert_called_with for node_similarity_search in the duplicate test uses the correct updated parameter order (driver, embedding, SearchFilters(), groups, limit).
  • Reason this comment was not posted:
    Comment looked like it was already resolved.

Workflow ID: wflow_tRd0YMnzm9WMT9e5


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@prasmussen15 prasmussen15 merged commit 088029a into main Feb 21, 2025
7 checks passed
@prasmussen15 prasmussen15 deleted the entity-type-search branch February 21, 2025 17:38
@github-actions github-actions bot locked and limited conversation to collaborators Feb 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant