-
Notifications
You must be signed in to change notification settings - Fork 460
chore: Update the context for date extraction + bug fixes #31
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
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 459afb5 in 30 seconds
More details
- Looked at
103
lines of code in4
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. core/graphiti.py:41
- Draft comment:
Removed unused importextract_node_edge_node_triplet
. This is a good cleanup step. - Reason this comment was not posted:
Confidence changes required:0%
The removal ofextract_node_edge_node_triplet
from imports ingraphiti.py
is correct since it's no longer used in the code.
2. core/graphiti.py:178
- Draft comment:
Simplified logic by directly usingexisting_edges + deduped_edges
foredges_to_save
. This is a good improvement. - Reason this comment was not posted:
Confidence changes required:0%
The change inadd_episode
method to useexisting_edges + deduped_edges
directly foredges_to_save
is correct and simplifies the logic.
3. core/prompts/extract_edge_dates.py:24
- Draft comment:
Removedsource_node
andtarget_node
from the prompt context. This aligns with the updated function signature and context usage. - Reason this comment was not posted:
Confidence changes required:0%
The removal ofsource_node
andtarget_node
from the prompt inextract_edge_dates.py
is consistent with the changes intemporal_operations.py
.
4. core/utils/maintenance/temporal_operations.py:126
- Draft comment:
Updatedextract_edge_dates
to useEntityEdge
directly instead ofNodeEdgeNodeTriplet
. This change is consistent with the updated logic. - Reason this comment was not posted:
Confidence changes required:0%
Theextract_edge_dates
function no longer usesNodeEdgeNodeTriplet
, which is consistent with the changes in the codebase.
Workflow ID: wflow_keh0zdzqDoEcZrUR
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 66df94a in 19 seconds
More details
- Looked at
14
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. core/utils/maintenance/temporal_operations.py:84
- Draft comment:
Consider usinglogger.debug
instead of removing the logger statements if they were used for debugging purposes. This helps in maintaining useful logs without cluttering production logs. - Reason this comment was not posted:
Confidence changes required:50%
The logger statements for debugging have been removed. If these were used for debugging purposes, consider using a debug level instead of info to avoid cluttering logs in production.
2. core/utils/maintenance/temporal_operations.py:84
- Draft comment:
Avoid logging sensitive data. Ensure that the context being logged does not contain sensitive information. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is not about a change made in the diff, as the logging line it refers to was removed. Additionally, the issue it raises is already resolved by the removal of the logging line. Therefore, the comment should be removed.
I might be missing the context of whether the 'context' variable still contains sensitive data that is logged elsewhere, but the comment is specifically about a removed line.
The comment is specifically about a removed line, so it should be removed regardless of other potential issues.
The comment should be removed because it is not about a change made in the diff and the issue it raises is already resolved.
Workflow ID: wflow_o3sCGHVIVNtsROsS
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Summary:
Updated date extraction context and fixed edge handling bugs in core files.
Key points:
core/graphiti.py
,core/prompts/extract_edge_dates.py
, andcore/utils/maintenance/temporal_operations.py
.extract_node_edge_node_triplet
fromcore/graphiti.py
andcore/utils/maintenance/temporal_operations.py
.extract_edge_dates
incore/utils/maintenance/temporal_operations.py
to useEntityEdge
directly.v1
function incore/prompts/extract_edge_dates.py
by removing source and target nodes.add_episode
incore/graphiti.py
to handle edge expiration and date extraction more efficiently.runner.py
by removing a commented-out line.Generated with ❤️ by ellipsis.dev