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

fix(agents-api): Fix search embeddings query to avoid snippets with embedding == null #662

Merged
merged 2 commits into from
Oct 15, 2024

Conversation

creatorrr
Copy link
Contributor

@creatorrr creatorrr commented Oct 15, 2024

Signed-off-by: Diwank Singh Tomer [email protected]


Important

Adds conditions in search_docs_by_embedding.py to exclude null embeddings, ensuring only valid embeddings are processed.

  • Behavior:
    • In search_docs_by_embedding in search_docs_by_embedding.py, added conditions to exclude snippets with embedding == null.
    • Ensures only valid embeddings are processed in the search query.
  • Misc:
    • Added candidate clause in search_docs_by_text.py for consistency.

This description was created by Ellipsis for 7decd8a. 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.

👍 Looks good to me! Reviewed everything up to 12978b4 in 9 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. agents-api/agents_api/models/docs/search_docs_by_embedding.py:112
  • Draft comment:
    The addition of is_null(embedding1) == false and is_null(embedding2) == false is a good practice to ensure that null embeddings are not processed, preventing potential runtime errors.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The added lines ensure that embeddings are not null before proceeding with distance calculations, which is a necessary check to prevent errors.

Workflow ID: wflow_GdTkYOG1pExvFLwL


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 7decd8a in 21 seconds

More details
  • Looked at 60 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. agents-api/agents_api/models/docs/search_docs_by_text.py:154
  • Draft comment:
    Consider adding a check for null embeddings similar to search_docs_by_embedding.py to ensure consistency and avoid potential issues with null embeddings in text-based searches.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_E6AEHWKTpLLuOHJU


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

Copy link
Contributor

@HamadaSalhab HamadaSalhab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@HamadaSalhab HamadaSalhab merged commit 994dbe0 into dev Oct 15, 2024
10 of 14 checks passed
@HamadaSalhab HamadaSalhab deleted the x/session-chat-broken branch October 15, 2024 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants