Skip to content

Conversation

@Strift
Copy link
Collaborator

@Strift Strift commented Feb 5, 2024

Pull Request

Hi 👋 this PR enables using the hybrid search experimental API with @meilisearch/instant-meilisearch.

Issue

Solves #1286

Proposed API

Allow users to pass a hybrid object in the meiliSearchParams object when creating an instantMeilisearch client.

instantMeiliSearch(host, apiKey, {
  primaryKey: 'id',
  
  meiliSearchParams: {
    // other meilisearch params
    hybrid: {
      semanticRatio: 0.9,
      embedder: 'default',
    },
  },
})

Additionally, the adaptSearchParams function will now internally handle this hybrid object.

What does this PR do?

  • Add hybrid object as an overridable search parameter

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

@changeset-bot
Copy link

changeset-bot bot commented Feb 5, 2024

🦋 Changeset detected

Latest commit: a2f3192

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@meilisearch/instant-meilisearch Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Strift Strift added the javascript Pull requests that update Javascript code label Feb 5, 2024
@Strift Strift changed the title Enable hybrid search InstantMeilisearch: Enable hybrid search Feb 6, 2024
@Strift Strift requested a review from brunoocasali February 6, 2024 10:46
Copy link
Member

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

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

Hi @Strift thanks a lot for this PR!

Although, we would need some tests to be able to merge it :)

Copy link
Member

Choose a reason for hiding this comment

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

Don't you need to make the types for the hybrid search available somewhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think so.

The HybridSearch type is only used internally in meilisearch (JS) in the SearchParams type.

The SearchParams type is exported, so users can still do:

const config: SearchParams[hybrid] = {} // configuration will have the correct type

NB: other internal types in SearchParams, like MatchingStrategies aren't exported either. So I think this is consistent.

meiliSearchParams.attributesToSearchOn = value
}
},
addHybridSearch() {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a test case for that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added a test! It seems to work, but maybe there's a different way to approach this. Let me know!

@curquiza curquiza added enhancement New feature or request and removed javascript Pull requests that update Javascript code labels Feb 6, 2024
@Strift Strift requested a review from brunoocasali February 7, 2024 11:10
Copy link
Member

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

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

LGTM!! bors merge

@brunoocasali brunoocasali merged commit 41830c7 into main Feb 7, 2024
@brunoocasali brunoocasali deleted the enable-hybrid-search branch February 7, 2024 13:49
@curquiza curquiza mentioned this pull request Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants