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

Experiments: Add possible suggestion to the empty results response #391

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

marko-bekhta
Copy link
Collaborator

@marko-bekhta marko-bekhta commented Jan 13, 2025

Fixes #306

Hey @yrodiere

I was going through some tickets at HSEARCH 😃 and noticed one submitted by the community about suggesters, but without much info. Hence, I thought I'd take a closer look at the suggesters before closing the ticket 🙈 and it turned out to be an interesting feature. This PR tests how we could leverage from it in search.quarkus.io.
While at it, I thought: should we give users a way to manipulate the request JSON before we send it (like in this PR we are adding another top-level element to the JSON, so adding things through e.g. a JSON predicate, wouldn't really work...)

image

Some links:

Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

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

The feature seems nice, +1 to add it.

A few comments below though :)

@marko-bekhta
Copy link
Collaborator Author

ok... played with it a bit more so it's more usable on the actual site, and user can click to get the results:

sugg

@marko-bekhta marko-bekhta force-pushed the test/suggestion branch 2 times, most recently from f11b6cb to 5a4161d Compare January 15, 2025 16:15
@marko-bekhta marko-bekhta marked this pull request as ready for review January 23, 2025 08:50
@yrodiere
Copy link
Member

ok... played with it a bit more so it's more usable on the actual site, and user can click to get the results:

Ready to merge, then? :)

@yrodiere
Copy link
Member

Also, should we mark this as fixing #306? @rsvoboda , is this enough to address your problem?

@marko-bekhta
Copy link
Collaborator Author

rebased the pr, but let's get the relevance PR in first (#402)

@rsvoboda
Copy link
Member

@marko-bekhta / @yrodiere is there a stage instance I could play with?

@yrodiere
Copy link
Member

@marko-bekhta / @yrodiere is there a stage instance I could play with?

There is, but it's based on main, so we need to merge this in order for you to play with it.

I guess we will soon enough, Marko is trying to get CI to pass :)

@marko-bekhta
Copy link
Collaborator Author

I guess we will soon enough, Marko is trying to get CI to pass :)

yeah 😄 those not sorted imports ... 😄

@rsvoboda
Copy link
Member

I checked the code and this could fix my #306 concern.

Maybe the tests could cover cases mentioned in the issue. Or I can add them later. It's up to you guys :)

@marko-bekhta
Copy link
Collaborator Author

Added a few more test queries, including the missing ones from the ticket description 😃; let's see if the build will pass on CI 🤞🏻

@marko-bekhta marko-bekhta merged commit 55518be into quarkusio:main Jan 29, 2025
1 check passed
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.

Enhancement proposal - be permissive about typos when searching
3 participants