-
Notifications
You must be signed in to change notification settings - Fork 159
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
Invalidation updates && improvements #20
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 7eb836e in 37 seconds
More details
- Looked at
645
lines of code in9
files - Skipped
0
files when reviewing. - Skipped posting
6
drafted comments based on config settings.
1. core/graphiti.py:138
- Draft comment:
Redundant logging statement for extracted nodes. This line duplicates the log at line 128 and can be removed to avoid confusion. - Reason this comment was not posted:
Confidence changes required:50%
The code incore/graphiti.py
has a redundant logging statement for extracted nodes. This is unnecessary and can be removed to avoid confusion and clutter in the logs.
2. core/prompts/dedupe_edges.py:74
- Draft comment:
Consider usingcontext['new_edges']
instead ofcontext['extracted_edges']
for consistency with the task description. - Reason this comment was not posted:
Confidence changes required:50%
Incore/prompts/dedupe_edges.py
, thev2
function has a minor issue where it usesjson.dumps
oncontext['extracted_edges']
instead ofcontext['new_edges']
. This could lead to confusion or errors if the context keys are not consistent.
3. core/prompts/invalidate_edges.py:38
- Draft comment:
The format string for edges should match the description provided in the prompt. Ensure the format includes both the fact and timestamp as described. - Reason this comment was not posted:
Confidence changes required:50%
Incore/prompts/invalidate_edges.py
, the prompt format for edges includes a fact and timestamp, but the format string in the code does not match this description. This could lead to inconsistencies in the LLM's understanding of the task.
4. core/utils/maintenance/edge_operations.py:227
- Draft comment:
Useexisting_edges
instead ofextracted_edges
when preparing the context for LLM to ensure correct deduplication results. - Reason this comment was not posted:
Confidence changes required:50%
Incore/utils/maintenance/edge_operations.py
, thededupe_extracted_edges_v2
function incorrectly usesextracted_edges
instead ofexisting_edges
when preparing the context for LLM. This could lead to incorrect deduplication results.
5. core/utils/maintenance/node_operations.py:162
- Draft comment:
The comment indicates a temporary hack. Consider refactoring this section for better maintainability and robustness. - Reason this comment was not posted:
Confidence changes required:50%
Incore/utils/maintenance/node_operations.py
, thededupe_extracted_nodes
function has a comment indicating a temporary hack. This suggests the code may not be robust or maintainable.
6. core/utils/search/search_utils.py:236
- Draft comment:
Consider adding exception handling insafely_parse_db_date
to handle potential parsing errors gracefully. - Reason this comment was not posted:
Confidence changes required:50%
Incore/utils/search/search_utils.py
, thesafely_parse_db_date
function should handle potential exceptions when parsing dates to ensure robustness.
Workflow ID: wflow_fM2zfqH6zm80Vd4u
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
…from-context # Conflicts: # core/graphiti.py # core/prompts/dedupe_edges.py # core/prompts/invalidate_edges.py # core/utils/bulk_utils.py # core/utils/maintenance/edge_operations.py # core/utils/maintenance/node_operations.py # core/utils/maintenance/temporal_operations.py # core/utils/search/search_utils.py # runner.py
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 80612c4 in 39 seconds
More details
- Looked at
1148
lines of code in6
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. core/graphiti.py:161
- Draft comment:
Remove commented-out code fordedupe_extracted_edges_v2
if it's not needed. - Reason this comment was not posted:
Confidence changes required:50%
The code incore/graphiti.py
has a potential issue with the commented-out code fordedupe_extracted_edges_v2
. The new deduplication functiondedupe_extracted_edges
is used instead, but the commented-out code should be removed if it's not needed.
Workflow ID: wflow_Ztp7sh16kl5ijjdK
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 03ad8cf in 14 seconds
More details
- Looked at
60
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. core/prompts/dedupe_edges.py:7
- Draft comment:
Inconsistent indentation: mix of tabs and spaces. Use one style consistently throughout the file. - Reason this comment was not posted:
Confidence changes required:50%
The indentation in the file is inconsistent, using both tabs and spaces. This can lead to readability issues and potential errors in some environments.
Workflow ID: wflow_OLCDOBolulMb7Bbl
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
…ti into extract-dates-from-context
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 6a97e9f in 13 seconds
More details
- Looked at
41
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. core/graphiti.py:183
- Draft comment:
Consider refactoring the deduplication and invalidation logic into a separate function to improve code reusability and readability. This pattern is repeated in multiple places. - Reason this comment was not posted:
Confidence changes required:50%
The code for deduplication and invalidation of edges is repeated in multiple places, which can lead to maintenance issues and potential bugs if changes are needed in the future. It would be better to refactor this into a separate function to improve code reusability and readability.
Workflow ID: wflow_HUXITS2ywikq2zJh
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
* wip * wip * wip * fix: Linter errors * fix formatting * chore: fix ruff * fix: Duplication --------- Co-authored-by: Daniel Chalef <131175+danielchalef@users.noreply.github.com>
Summary:
This PR refines deduplication and invalidation processes in the graph database and
core/graphiti.py
, prioritizing invalidated edges and enhancing semantic similarity handling.Key points:
core/graphiti.py
.add_episode
.dedupe_extracted_edges_v2
for improved edge deduplication.add_episode
for better node extraction and edge invalidation.extract_node_and_edge_triplets
for enhanced edge processing.invalidate_edges
to include episode context.safely_parse_db_date
for better date handling from Neo4j.core/prompts/dedupe_edges.py
to refine edge deduplication logic inv1
,v2
, andedge_list
functions.Generated with ❤️ by ellipsis.dev