Skip to content

Conversation

@jennapederson
Copy link
Contributor

Problem

Added:

  • lexical search notebook
  • cascading retrieval notebook

Solution

Describe the approach you took. Link to any relevant bugs, issues, docs, or other resources.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Infrastructure change (CI configs, etc)
  • Non-code change (docs, etc)
  • None of the above: (explain here)

Test Plan

Describe specific steps for validating this change.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@@ -0,0 +1,680 @@
{
Copy link
Contributor

@arjunpatel7 arjunpatel7 Aug 15, 2025

Choose a reason for hiding this comment

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

Line #1.    def print_hits(results):

Some comments here would be helpful!


Reply via ReviewNB

@@ -0,0 +1,680 @@
{
Copy link
Contributor

@arjunpatel7 arjunpatel7 Aug 15, 2025

Choose a reason for hiding this comment

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

Line #6.        api_key=api_key,

add text on removing source tag for prod/self


Reply via ReviewNB

@@ -0,0 +1,680 @@
{
Copy link
Contributor

@arjunpatel7 arjunpatel7 Aug 15, 2025

Choose a reason for hiding this comment

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

You should mention here that if you do this reranking, the responses might not be ordered on on their lexical relevance along anymore. I.E, it's possible to have an ordering of lexical top to bottom, and that get's a bit muddled after reranking semantically, but relevance is maximized nonetheless.


Reply via ReviewNB

@arjunpatel7
Copy link
Contributor

LGTM, but added some comments that could help clarify

@jennapederson jennapederson marked this pull request as ready for review August 15, 2025 17:38
@jennapederson jennapederson merged commit 663f0fe into master Aug 15, 2025
17 of 18 checks passed
@jennapederson jennapederson deleted the search-notebooks branch August 15, 2025 19:05
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.

3 participants