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

Mentions reranker #124

Merged
merged 9 commits into from
Sep 18, 2024
Merged

Mentions reranker #124

merged 9 commits into from
Sep 18, 2024

Conversation

prasmussen15
Copy link
Collaborator

@prasmussen15 prasmussen15 commented Sep 18, 2024

🚀 This description was created by Ellipsis for commit 3781c7f

feat: add episode_mentions reranker and update search configurations

Summary:

Add episode_mentions reranker, update search configurations, and enhance edge handling and retrieval methods.

Key points:

  • Add episode_mentions reranker to EdgeReranker and NodeReranker in search_config.py.
  • Implement episode_mentions_reranker() in search_utils.py.
  • Update edge_search() and node_search() in search.py.
  • Add EDGE_HYBRID_SEARCH_EPISODE_MENTIONS and NODE_HYBRID_SEARCH_EPISODE_MENTIONS in search_config_recipes.py.
  • Add get_by_uuids() method to EpisodicEdge, EntityEdge, and CommunityEdge in edges.py.
  • Modify dedupe_extracted_edges() in edge_operations.py.
  • Add get_episode_mentions() in graphiti.py.
  • Update get_by_uuids() in nodes.py.

Generated with ❤️ by ellipsis.dev

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 3b18716 in 32 seconds

More details
  • Looked at 379 lines of code in 9 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. graphiti_core/edges.py:313
  • Draft comment:
    Consider adding a check for empty edges list and raising EdgeNotFoundError similar to other get_by_uuids methods.
  • Reason this comment was not posted:
    Confidence changes required: 70%
    The get_by_uuids method in CommunityEdge class does not handle the case where no edges are found, unlike the other similar methods.
2. graphiti_core/nodes.py:173
  • Draft comment:
    The use of DISTINCT in the get_by_uuids method is unnecessary and should be removed to avoid confusion.
  • Reason this comment was not posted:
    Confidence changes required: 70%
    The get_by_uuids method in EpisodicNode class should not use DISTINCT as it is not necessary and could lead to confusion.
3. graphiti_core/search/search_utils.py:73
  • Draft comment:
    Missing comma in the RETURN clause after c.name_embedding AS name_embedding. This issue is also present in the get_mentioned_nodes function.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    In get_mentioned_nodes and get_communities_by_nodes, there is a missing comma in the RETURN clause.
4. graphiti_core/search/search_utils.py:669
  • Draft comment:
    The comment about finding the shortest path is misleading. The function counts relationships, not paths. Consider updating the comment for clarity.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    In the episode_mentions_reranker function, the comment about shortest path is misleading as the function is counting relationships, not finding paths.

Workflow ID: wflow_8qAGRi2yBb9a3WmJ


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.

graphiti_core/edges.py Outdated Show resolved Hide resolved
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 3781c7f in 21 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. graphiti_core/edges.py:117
  • Draft comment:
    Typo in SQL query: INN should be IN. This typo is corrected in the PR, but ensure similar issues are not present elsewhere.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment correctly identifies a change made in the diff, which is the correction of a typo in the SQL query. However, the suggestion to ensure similar issues are not present elsewhere is speculative and not actionable based on the current diff. The comment should focus only on the change made, which is the correction of the typo.
    The comment does address a change made in the diff, but the additional suggestion to check for similar issues elsewhere is not actionable and speculative.
    The main focus should be on the correction made in the diff, which is valid. The speculative part of the comment should be disregarded.
    Keep the comment but remove the speculative part about ensuring similar issues are not present elsewhere.

Workflow ID: wflow_gofpqcoxOgL16Qy7


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

@prasmussen15 prasmussen15 merged commit e398f95 into main Sep 18, 2024
6 checks passed
@prasmussen15 prasmussen15 deleted the mentions-reranker branch September 18, 2024 19:44
@github-actions github-actions bot locked and limited conversation to collaborators Sep 18, 2024
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.

2 participants