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 Distance Reranker: Limit max hops (and cleanup prints) #72

Merged
merged 2 commits into from
Sep 1, 2024

Conversation

danielchalef
Copy link
Member

@danielchalef danielchalef commented Sep 1, 2024

This pull request includes two changes. First, it limits the maximum number of hops in the node distance reranker. Second, it cleans up unnecessary print statements in the code. These changes improve the efficiency and readability of the code.


🚀 This description was created by Ellipsis for commit b9f43bf

Summary:

Limits max hops in node_distance_reranker and removes unnecessary print statements for improved efficiency and readability.

Key points:

  • graphiti_core/search/search_utils.py: Limits max hops to 10 in node_distance_reranker for optimized search.
  • graphiti_core/llm_client/openai_client.py: Removes print statement in _generate_response.
  • graphiti_core/utils/maintenance/edge_operations.py: Removes print statement in extract_edges.

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 b9f43bf in 16 seconds

More details
  • Looked at 39 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. graphiti_core/llm_client/openai_client.py:63
  • Draft comment:
    Consider using logger.debug(openai_messages) if you need to log the openai_messages for debugging purposes instead of removing the print statement entirely.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR removes unnecessary print statements, which is a good practice for cleaner code. However, the logger should be used for debugging purposes instead of print statements.
2. graphiti_core/utils/maintenance/edge_operations.py:72
  • Draft comment:
    Consider using logger.debug(llm_response) if you need to log the llm_response for debugging purposes instead of removing the print statement entirely.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR removes unnecessary print statements, which is a good practice for cleaner code. However, the logger should be used for debugging purposes instead of print statements.

Workflow ID: wflow_jBWY8H036HfNRyhp


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.

@danielchalef danielchalef changed the title Limit max hops and cleanup prints Node Distance Reranker: Limit max hops (and cleanup prints) Sep 1, 2024
@danielchalef danielchalef merged commit fe20c0f into main Sep 1, 2024
6 checks passed
@danielchalef danielchalef deleted the fix/node-dist-search-optimization branch September 1, 2024 19:16
@github-actions github-actions bot locked and limited conversation to collaborators Sep 1, 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